Fix a narrowing conversion#964
Conversation
OmniBlade
left a comment
OmniBlade
left a comment
There was a problem hiding this comment.
Comments apply to both games code, seems like a lot of changes to just silence a warning?
| */ | ||
| #ifdef VIRGIN_CHEAT_KEYS | ||
| #define PARM_PLAYTEST 0xF7DDC227 // "PLAYTEST" | ||
| #define PARM_PLAYTEST 0xF7DDC227UL // "PLAYTEST" |
There was a problem hiding this comment.
uint32_t is not unsigned long on all platforms.
| { | ||
| Target.Sub.Exponent = RTTI_NONE; | ||
| Target.Sub.Mantissa = -1; | ||
| Target.Sub.Mantissa = (1UL << TARGET_MANTISSA) - 1; |
There was a problem hiding this comment.
I don't think this is equivalent?
| * 08/19/1995 JLB : Created. * | ||
| *=============================================================================================*/ | ||
| long Obfuscate(char const* string) | ||
| uint32_t Obfuscate(char const* string) |
There was a problem hiding this comment.
The long type needs to go as its width varies between platforms but it should just change to int to match the ra code?
|
It's not only to get rid of the warning. Obfuscate() currently operates on an int, but in TD returns a long (unlike RA, which currently returns an int). Comparing that to a constant like 0xF7DDC227 may fail if the function actually returned 0xFFFFFFFFF7DDC227 because it was sign-extended to long. The only safe way is to declare them to return unsigned int (or uint32_t in this case), since this is what they actually calculate. |
|
I'd be happy to see it changed to unsigned int in both cases. |
OmniBlade
left a comment
OmniBlade
left a comment
There was a problem hiding this comment.
Most of the change made in this PR have been merged as part of other work, the remaining fixes for target mantissa still look useful however.
2 participants
No description provided.