Skip to content

Conversation

@Skyaero42
Copy link

@Skyaero42 Skyaero42 commented Oct 10, 2025

This change replaces strcat with strlcat for robustness

TODO

  • Replicate in Generals

@Skyaero42 Skyaero42 added this to the Code foundation build up milestone Oct 10, 2025
@Skyaero42 Skyaero42 self-assigned this Oct 10, 2025
@Skyaero42 Skyaero42 added the Stability Concerns stability of the runtime label Oct 10, 2025
@Skyaero42 Skyaero42 changed the title ix: replace strcat with strlcat for robustness fix: replace strcat with strlcat for robustness Oct 10, 2025
@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing labels Oct 12, 2025
@xezon xezon changed the title fix: replace strcat with strlcat for robustness fix: Replace strcat with strlcat for robustness Oct 12, 2025
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Note that strlcpy, strlcat will have a tiny computational overhead over strcpy, strcat, for the benefit of stability. When touching hot code, such as AsciiString, UnicodeString, be extra mindful about it.

Generally, prefer using ARRAY_SIZE(buffer) instead of sizeof(buffer) or BUFFER_SIZE_VALUE. Because ARRAY_SIZE(buffer) works for both char* and wchar_t* and adapts without touching when the actual buffer changes.

Examples:

wchar_t buffer[123];
wcslcpy(buffer, src, sizeof(buffer)); // <---- wrong! sizeof(buffer) is too large and allows buffer overflow: better ARRAY_SIZE(buffer)
char buffer[MAX_PATH];
strlcpy(buffer, src, MAX_PATH); // <---- correct, but needs to be kept in sync with declaration: better ARRAY_SIZE(buffer)

@Skyaero42 Skyaero42 force-pushed the skyaero/fix-strcat-to-strlcat branch from 073cb7f to 02fdae5 Compare October 14, 2025 20:05
@Skyaero42
Copy link
Author

I don't understand why it doesn't compile VC6. Works fine on my PC.

@xezon
Copy link

xezon commented Oct 15, 2025

Try change return(bool(value)); to return (bool)value;

@Skyaero42 Skyaero42 marked this pull request as draft October 15, 2025 08:47
@Skyaero42 Skyaero42 force-pushed the skyaero/fix-strcat-to-strlcat branch 2 times, most recently from 415e947 to 0537c28 Compare October 15, 2025 09:14
@Skyaero42 Skyaero42 marked this pull request as ready for review October 15, 2025 09:14
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Needs to be replicated to Generals.

@Skyaero42
Copy link
Author

Rebased and replicated in generals

@Skyaero42 Skyaero42 force-pushed the skyaero/fix-strcat-to-strlcat branch from 0537c28 to 14e6641 Compare October 15, 2025 18:24
@xezon xezon merged commit 47b4cdc into TheSuperHackers:main Oct 15, 2025
17 checks passed
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing Stability Concerns stability of the runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants