[bobtista]

• Follow up for refactor(pathutil): Add appendPathSeparator for character buffers #3444
• Merge after refactor(pathutil): Add appendPathSeparator for character buffers #3444

DirectorySelectProc builds each folder to add with sprintf( toAdd, "%s%s\\", buffer, text ). buffer holds the current directory and toAdd is _MAX_PATH long, so a long current directory plus a subfolder name writes past toAdd. When the current directory leaves no room for its trailing separator, the subfolder name is also joined to it without one.

Now the path is built with snprintf, and a folder whose path does not fit is skipped with a message. A current directory with no room for its separator always makes toAdd overflow, so the same check covers both cases.

Reproduction:

1. Start ImagePacker.
2. Open the add folder dialog.
3. Navigate into a directory whose path is close to 259 characters and has a subfolder.
4. Select the subfolder and press Add.

Todo:

• Build win32 and vc6
• Test the add folder dialog with a long directory path

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 791f588
• Latest commit, not reviewed: a9a7d50

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The change adds a bounded path-separator helper and uses it in engine code and tools. Call sites return early when the path cannot be extended. ImagePacker also checks whether a selected folder path fits its buffer before adding it.

Changes

Path separator handling

Layer / File(s)	Summary
Add bounded separator helper Core/Libraries/Source/Lib/Include/Lib/PathUtil.h	appendPathSeparator leaves empty or already-delimited paths unchanged. Otherwise, it appends the native separator and null terminator if the buffer has space, and returns false if it does not.
Normalize library directory paths Core/Libraries/Source/WWVegas/WW3D2/agg_def.cpp, Core/Libraries/Source/WWVegas/WWLib/ffactory.cpp	Asset loading and subdirectory handling use appendPathSeparator. The subdirectory methods recalculate the path length after appending.
Normalize tool directory paths Core/Tools/Autorun/Wnd_file.cpp, Core/Tools/ImagePacker/Source/WindowProcedures/DirectorySelect.cpp, Core/Tools/W3DView/Utils.h, Core/Tools/W3DView/W3DViewDoc.cpp, Generals/Code/Tools/WorldBuilder/src/*, GeneralsMD/Code/Tools/WorldBuilder/src/*	The tools use shared separator handling for directory paths. ImagePacker uses bounded formatting for selected folder paths and skips paths that do not fit.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

• TheSuperHackers/GeneralsGameCode#3598: Refactors PathUtil.h separator interfaces used by the helper in this change.

Suggested labels: Fix, Minor, Platform

Suggested reviewers: xezon

Merge Risk: Moderate · previous review 791f5

Resolve the remaining long-path and settings-path failures before merging to avoid crashes or writes to incorrect locations.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the ImagePacker bug fix and the handling of folder paths that exceed the add-folder buffer. It matches the primary objective of the changeset.
Description check		The description explains the buffer overflow, the missing-separator case, the snprintf-based fix, the skip behavior, reproduction steps, and testing status. It is directly related to the changeset.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 593cdef2-a43c-4435-a4d6-19a0f1ac6920

📥 Commits

Reviewing files that changed from the base of the PR and between 2886271 and d85c07c.

📒 Files selected for processing: 89

• Core/GameEngine/Include/GameNetwork/FileTransfer.h
• Core/GameEngine/Source/Common/Audio/AudioEventRTS.cpp
• Core/GameEngine/Source/Common/CRCDebug.cpp
• Core/GameEngine/Source/Common/CommandLine.cpp
• Core/GameEngine/Source/Common/INI/INI.cpp
• Core/GameEngine/Source/Common/INI/INIMapCache.cpp
• Core/GameEngine/Source/Common/MiniLog.cpp
• Core/GameEngine/Source/Common/ReplaySimulation.cpp
• Core/GameEngine/Source/Common/System/BIGFileSystem.cpp
• Core/GameEngine/Source/Common/System/FileSystem.cpp
• Core/GameEngine/Source/Common/System/GameMemoryInit.cpp
• Core/GameEngine/Source/Common/WorkingDirectory.cpp
• Core/GameEngine/Source/GameClient/GUI/GameWindowManagerScript.cpp
• Core/GameEngine/Source/GameClient/MapUtil.cpp
• Core/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
• Core/GameEngine/Source/GameNetwork/FileTransfer.cpp
• Core/GameEngine/Source/GameNetwork/GameInfo.cpp
• Core/GameEngine/Source/GameNetwork/GameSpy/LobbyUtils.cpp
• Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp
• Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
• Core/Libraries/Source/Lib/Debug.cpp
• Core/Libraries/Source/Lib/Include/Lib/PathUtil.h
• Core/Libraries/Source/WWVegas/WW3D2/agg_def.cpp
• Core/Libraries/Source/WWVegas/WW3D2/ringobj.cpp
• Core/Libraries/Source/WWVegas/WW3D2/sphereobj.cpp
• Core/Libraries/Source/WWVegas/WW3D2/w3d_dep.cpp
• Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp
• Core/Libraries/Source/WWVegas/WWAudio/Utils.h
• Core/Libraries/Source/WWVegas/WWDownload/FTP.cpp
• Core/Libraries/Source/WWVegas/WWLib/WWCommon.h
• Core/Libraries/Source/WWVegas/WWLib/ffactory.cpp
• Core/Tools/Autorun/Utils.cpp
• Core/Tools/Autorun/Wnd_file.cpp
• Core/Tools/ImagePacker/Source/ImagePacker.cpp
• Core/Tools/ImagePacker/Source/WindowProcedures/DirectorySelect.cpp
• Core/Tools/Launcher/DatGen/DatGen.cpp
• Core/Tools/Launcher/findpatch.cpp
• Core/Tools/Launcher/main.cpp
• Core/Tools/Launcher/patch.cpp
• Core/Tools/W3DView/GraphicView.cpp
• Core/Tools/W3DView/MainFrm.cpp
• Core/Tools/W3DView/SaveSettingsDialog.cpp
• Core/Tools/W3DView/Utils.cpp
• Core/Tools/W3DView/Utils.h
• Core/Tools/W3DView/W3DViewDoc.cpp
• Core/Tools/WW3D/max2w3d/w3dutil.cpp
• Dependencies/Utility/Utility/stringex.h
• Generals/Code/GameEngine/Source/Common/Recorder.cpp
• Generals/Code/GameEngine/Source/Common/StatsCollector.cpp
• Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/DownloadMenu.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/GameInfoWindow.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanGameOptionsMenu.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupSaveLoad.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp
• Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp
• Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
• Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp
• Generals/Code/Tools/GUIEdit/Source/GUIEdit.cpp
• Generals/Code/Tools/WorldBuilder/src/BuildList.cpp
• Generals/Code/Tools/WorldBuilder/src/SaveMap.cpp
• Generals/Code/Tools/WorldBuilder/src/ScriptDialog.cpp
• Generals/Code/Tools/WorldBuilder/src/TerrainMaterial.cpp
• Generals/Code/Tools/WorldBuilder/src/TerrainModal.cpp
• Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp
• Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp
• GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp
• GeneralsMD/Code/GameEngine/Source/Common/StatsCollector.cpp
• GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/DownloadMenu.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/GameInfoWindow.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanGameOptionsMenu.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupSaveLoad.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/WOLGameSetupMenu.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp
• GeneralsMD/Code/Tools/GUIEdit/Source/GUIEdit.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/BuildList.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/MeshMoldOptions.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/SaveMap.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/TerrainMaterial.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/TerrainModal.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp
• GeneralsMD/Code/Tools/wdump/wdump.cpp

💤 Files with no reviewable changes: 3

• Core/GameEngine/Source/Common/System/FileSystem.cpp
• Core/Libraries/Source/WWVegas/WWAudio/Utils.h
• Core/GameEngine/Include/GameNetwork/FileTransfer.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Handle appendPathSeparator failure before building the settings path. · W3DViewDoc.cpp:1449

Core/Tools/W3DView/W3DViewDoc.cpp:1449
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle appendPathSeparator failure before building the settings path.

When the current directory length is MAX_PATH - 1, appendPathSeparator returns false and leaves szPath unchanged. The following CString concatenation does not overflow szPath, but it creates a path without a separator. The profile writes can therefore target the wrong or unusable path, while SaveSettings still returns TRUE unconditionally.

Suggested fix

-            appendPathSeparator (szPath, ARRAY_SIZE (szPath));
+            if (!appendPathSeparator (szPath, ARRAY_SIZE (szPath)))
+            {
+                return bReturn;
+            }

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: cb0ea6e5-3ac7-4355-a6c4-3ad1ac13ca7c

📥 Commits

Reviewing files that changed from the base of the PR and between d85c07c and eca20d8.

📒 Files selected for processing: 11

• Core/GameEngine/Source/GameClient/GUI/GameWindowManagerScript.cpp
• Core/Libraries/Source/Lib/Debug.cpp
• Core/Libraries/Source/Lib/Include/Lib/PathUtil.h
• Core/Libraries/Source/debug/debug_debug.cpp
• Core/Libraries/Source/debug/debug_io_flat.cpp
• Core/Libraries/Source/debug/debug_stack.cpp
• Core/Tools/W3DView/W3DViewDoc.cpp
• Generals/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp
• Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp
• GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Bound the duplicate-folder message. · DirectorySelect.cpp:210

Core/Tools/ImagePacker/Source/WindowProcedures/DirectorySelect.cpp:210
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Bound the duplicate-folder message.

The new check accepts a toAdd length of 256–259 characters. If the user adds that folder again, this sprintf overflows message: the fixed text needs 36 characters, but the buffer reserves only 32 beyond _MAX_PATH. Use bounded formatting for this message too.

Based on learnings: fixed-size path buffers must reserve space for appended characters and the terminator.

Source: Learnings

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5663af13-3d32-48a4-8dca-41c1f58d8dc2

Commits

Reviewing files that changed from the base of the PR and between eca20d8 and 791f588.

Files selected for processing: 11

• Core/Libraries/Source/Lib/Include/Lib/PathUtil.h
• Core/Libraries/Source/WWVegas/WW3D2/agg_def.cpp
• Core/Libraries/Source/WWVegas/WWLib/ffactory.cpp
• Core/Tools/Autorun/Wnd_file.cpp
• Core/Tools/ImagePacker/Source/WindowProcedures/DirectorySelect.cpp
• Core/Tools/W3DView/Utils.h
• Core/Tools/W3DView/W3DViewDoc.cpp
• Generals/Code/Tools/WorldBuilder/src/SaveMap.cpp
• Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/SaveMap.cpp
• GeneralsMD/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[bobtista]

Bound the duplicate-folder message.

The duplicate folder message now uses snprintf too.