fix: Replace strncpy with strlcpy for robustness #1533

Skyaero42 · 2025-09-04T07:23:41Z

Related to Replace strcpy, strncpy & co with strlcpy family functions #1518

Generals/Code/Libraries/Source/WWVegas/WW3D2/hmorphanim.cpp

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp

xezon · 2025-09-04T08:56:50Z

"Partially resolves" means merging this change will close the issue.

xezon

This needs another pass. strncpy was not correctly substituted at many places.

Core/Libraries/Source/WWVegas/WW3D2/w3d_dep.cpp

Core/Libraries/Source/WWVegas/WW3D2/hmdldef.cpp

Core/Libraries/Source/WWVegas/WWDownload/FTP.CPP

Core/Libraries/Source/WWVegas/WWLib/ini.cpp

helmutbuhler · 2025-09-04T22:04:24Z

The Replay Checker crashes with this:
GetStringFromRegistry - looking in SOFTWARE\Electronic Arts\EA Games\General for key InstallPat
Looks like the last character is deleted.

Core/GameEngine/Source/Common/System/AsciiString.cpp

Core/Libraries/Source/WWVegas/WW3D2/w3d_dep.cpp

xezon

I think the lengths need another review pass.

Core/Libraries/Source/WWVegas/WW3D2/w3d_dep.cpp

Core/Libraries/Source/WWVegas/WWLib/ini.cpp

Core/Libraries/Source/debug/debug_io_flat.cpp

Core/Tools/ImagePacker/Source/ImagePacker.cpp

Generals/Code/GameEngine/Source/Common/Recorder.cpp

Generals/Code/Libraries/Source/WWVegas/WW3D2/hmorphanim.cpp

Generals/Code/Libraries/Source/WWVegas/WW3D2/meshmdlio.cpp

Generals/Code/Tools/WorldBuilder/src/BlendMaterial.cpp

Generals/Code/Tools/WorldBuilder/src/TerrainMaterial.cpp

Generals/Code/Tools/WorldBuilder/src/TerrainModal.cpp

Generals/Code/GameEngine/Source/GameNetwork/LANAPI.cpp

GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp

Generals/Code/GameEngine/Source/GameNetwork/LANAPI.cpp

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DProjectedShadow.cpp

GeneralsMD/Code/GameEngine/Source/GameNetwork/GameSpy/Thread/BuddyThread.cpp

GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DProjectedShadow.cpp

xezon · 2025-09-24T08:09:03Z

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DProjectedShadow.cpp

-	nameLen=strlen(shadowInfo->m_ShadowName);
-	if (nameLen <= 1)	//no texture name given, use same as object
-	{	strcpy(texture_name,defaultDecalName);
+	if (strlen(shadowInfo->m_ShadowName) <= 1)	//no texture name given, use same as object


I expect we can simplify this to if (shadowInfo->m_ShadowName[0] == '\0') without any change in functionality.

If I understand C-strings correctly:

m_ShadowName[0] = 'a'; m_ShadowName[1] = '\0';

This represents a string of length 1. In that case, strlen(m_ShadowName) returns 1, so the original condition strlen(...) <= 1 would evaluate to true.

The null terminator is at index [1], not [0]. So checking only m_ShadowName[0] == '\0' is not equivalent — it would only detect the empty string (""), not a one-character string.

So at the moment the game does not allow texture names of 1 character. I don't know if that is intentional, or that this comparison is bugged.

Yes it is odd comparison. One would expect strlen(shadowInfo->m_ShadowName) == 0 or strlen(shadowInfo->m_ShadowName) < 1. I suspect that was the intention.

You can run the game with a break point for if (strlen(shadowInfo->m_ShadowName) == 1) and see if that ever hits. If it never hits, I do not think we need to care for it.

Skyaero42 · 2025-09-30T06:36:10Z

Something is off and it is caused by this PR.

Spy satelite / spy drone have a similar issue

xezon · 2025-09-30T07:13:49Z

Looks shadow decal related

Skyaero42 · 2025-10-07T06:34:10Z

Made another pass

rebased onto latest main
debug/breakpoint on all statement to verify the correct copy. Unfortunately I couldn't trigger all statements - some functions may not be in use anymore
Applied a few fixes (in particular the shadow decal)

Game looks and runs well. I think this is (finally) it.

Core/Libraries/Source/profile/CMakeLists.txt

Core/Libraries/Source/debug/CMakeLists.txt

Core/Libraries/Source/profile/profile_highlevel.cpp

GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/meshmdlio.cpp

xezon · 2025-10-15T16:30:53Z

Is this fully replicated in Generals?

Skyaero42 · 2025-10-15T17:38:32Z

Is this fully replicated in Generals?

Yes it is

Skyaero42 mentioned this pull request Sep 4, 2025

Replace strcpy, strncpy & co with strlcpy family functions #1518

Open

11 tasks

Skyaero42 commented Sep 4, 2025

View reviewed changes

Generals/Code/Libraries/Source/WWVegas/WW3D2/hmorphanim.cpp Show resolved Hide resolved

Skyaero42 commented Sep 4, 2025

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLQuickMatchMenu.cpp Show resolved Hide resolved

Skyaero42 force-pushed the refractor/strncpy branch 4 times, most recently from 1b26f0d to 863e100 Compare September 4, 2025 07:48

xezon reviewed Sep 4, 2025

View reviewed changes

Skyaero42 force-pushed the refractor/strncpy branch from 863e100 to b035767 Compare September 4, 2025 19:08

Skyaero42 commented Sep 5, 2025

View reviewed changes

Core/GameEngine/Source/Common/System/AsciiString.cpp Outdated Show resolved Hide resolved

Skyaero42 commented Sep 5, 2025

View reviewed changes

Core/Libraries/Source/WWVegas/WW3D2/w3d_dep.cpp Outdated Show resolved Hide resolved

Skyaero42 force-pushed the refractor/strncpy branch from b035767 to 1c95020 Compare September 6, 2025 17:59

Skyaero42 marked this pull request as draft September 6, 2025 18:03

Skyaero42 self-assigned this Sep 6, 2025

Skyaero42 added the Refactor Edits the code with insignificant behavior changes, is never user facing label Sep 6, 2025

Skyaero42 marked this pull request as ready for review September 6, 2025 18:20

Skyaero42 force-pushed the refractor/strncpy branch from 1c95020 to a7561fd Compare September 7, 2025 18:38

Skyaero42 requested a review from xezon September 7, 2025 18:39

xezon changed the title ~~refactor: Replacing strncpy with strlcpy~~ fix: Replace strncpy with strlcpy for robustness Sep 8, 2025

xezon added this to the Stability fixes milestone Sep 8, 2025

xezon added Fix Is fixing something, but is not user facing Minor Severity: Minor < Major < Critical < Blocker labels Sep 8, 2025

xezon reviewed Sep 8, 2025

View reviewed changes

slurmlord reviewed Sep 9, 2025

View reviewed changes

Generals/Code/GameEngine/Source/GameNetwork/LANAPI.cpp Show resolved Hide resolved

slurmlord reviewed Sep 10, 2025

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp Outdated Show resolved Hide resolved

Skyaero42 force-pushed the refractor/strncpy branch 3 times, most recently from 18edc27 to bad3f0f Compare September 15, 2025 19:35

xezon reviewed Sep 16, 2025

View reviewed changes

Skyaero42 force-pushed the refractor/strncpy branch from bad3f0f to 606f5d1 Compare September 18, 2025 19:22

xezon reviewed Sep 19, 2025

View reviewed changes

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DProjectedShadow.cpp Outdated Show resolved Hide resolved

xezon reviewed Sep 24, 2025

View reviewed changes

Bart Roossien added 4 commits October 6, 2025 20:22

refactor: Replacing strncpy with strlcpy

08ee57b

changing sizeof() back to ARRAY_Size

ec72413

Essentially this is a strcpy now.

c4433a0

Some fixes

bd1b0e8

Skyaero42 force-pushed the refractor/strncpy branch from 5c11d68 to bd1b0e8 Compare October 7, 2025 06:27

xezon reviewed Oct 12, 2025

View reviewed changes

xezon added Stability Concerns stability of the runtime and removed Fix Is fixing something, but is not user facing labels Oct 12, 2025

Fixes based on feedback

1d5ba5c

xezon approved these changes Oct 15, 2025

View reviewed changes

xezon merged commit 630f6df into TheSuperHackers:main Oct 15, 2025
17 checks passed

fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Nov 10, 2025

fix: Replace strncpy with strlcpy for robustness (TheSuperHackers#1533)

4fdcf91

fix: Replace strncpy with strlcpy for robustness #1533

fix: Replace strncpy with strlcpy for robustness #1533

Uh oh!

Conversation

Skyaero42 commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon commented Sep 4, 2025

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

helmutbuhler commented Sep 4, 2025

Uh oh!

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

xezon Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

Skyaero42 commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon commented Sep 30, 2025

Uh oh!

Skyaero42 commented Oct 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon commented Oct 15, 2025

Uh oh!

Skyaero42 commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Skyaero42 commented Sep 4, 2025 •

edited

Loading

Skyaero42 commented Sep 30, 2025 •

edited

Loading