Skip to content

Fix a narrowing conversion#964

Open
th-otto wants to merge 1 commit into
TheAssemblyArmada:vanillafrom
th-otto:PR-16
Open

Fix a narrowing conversion#964
th-otto wants to merge 1 commit into
TheAssemblyArmada:vanillafrom
th-otto:PR-16

Conversation

@th-otto
Copy link
Copy Markdown
Contributor

@th-otto th-otto commented Mar 20, 2024

No description provided.

Copy link
Copy Markdown
Contributor

@OmniBlade OmniBlade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments apply to both games code, seems like a lot of changes to just silence a warning?

Comment thread redalert/defines.h
*/
#ifdef VIRGIN_CHEAT_KEYS
#define PARM_PLAYTEST 0xF7DDC227 // "PLAYTEST"
#define PARM_PLAYTEST 0xF7DDC227UL // "PLAYTEST"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32_t is not unsigned long on all platforms.

Comment thread redalert/target.h
{
Target.Sub.Exponent = RTTI_NONE;
Target.Sub.Mantissa = -1;
Target.Sub.Mantissa = (1UL << TARGET_MANTISSA) - 1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is equivalent?

Comment thread tiberiandawn/init.cpp
* 08/19/1995 JLB : Created. *
*=============================================================================================*/
long Obfuscate(char const* string)
uint32_t Obfuscate(char const* string)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long type needs to go as its width varies between platforms but it should just change to int to match the ra code?

@th-otto
Copy link
Copy Markdown
Contributor Author

th-otto commented Apr 15, 2024

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.

@OmniBlade
Copy link
Copy Markdown
Contributor

I'd be happy to see it changed to unsigned int in both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants