Skip to content

fix(ww3d2): Copy string setter inputs before freeing potentially aliased storage - #3313

Merged
xezon merged 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:bugfix/clang-tidy-string-setter-aliasing
Sep 20, 2026
Merged

xezon merged 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:bugfix/clang-tidy-string-setter-aliasing

Conversation

@CryoTheRenegade

Copy link
Copy Markdown

Fix several string setters that freed the owned buffer before copying the new value, which is unsafe when the input aliases the current storage (e.g. self-assignment / overlapping pointers).
Duplicate first, then free, in AggregateDefClass::Set_Name, ParameterClass::Set_Name, ParticleEmitterClass::Set_Name, and ParticleEmitterDefClass::Set_Name / Set_User_String.

Found by clang-tidy

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bf964c48-6d79-4d14-ad11-88498152ef7a

📥 Commits

Reviewing files that changed from the base of the PR and between 8605738 and 5b93732.

📒 Files selected for processing (6)
  • Core/Libraries/Source/WWVegas/WW3D2/agg_def.h
  • Core/Libraries/Source/WWVegas/WWSaveLoad/parameter.h
  • Generals/Code/Libraries/Source/WWVegas/WW3D2/part_emt.cpp
  • Generals/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp
  • GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_emt.cpp
  • GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

Changes

String replacement ordering

Layer / File(s) Summary
Core string setter updates
Core/Libraries/Source/WWVegas/WW3D2/agg_def.h, Core/Libraries/Source/WWVegas/WWSaveLoad/parameter.h
Core name setters now duplicate replacement strings before releasing existing allocations.
Particle emitter string setter updates
Generals/Code/Libraries/Source/WWVegas/WW3D2/part_emt.cpp, Generals/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp, GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_emt.cpp, GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/part_ldr.cpp
Particle emitter name and user-string setters now duplicate replacement strings before releasing existing values. Null replacement behavior remains supported.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 5b937

The string replacement changes preserve aliased inputs without introducing an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: string setters now copy inputs before freeing potentially aliased storage.
Description check ✅ Passed The description directly explains the affected setters, the aliasing issue, the fix, and the clang-tidy finding.

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

@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge and correctly addresses aliased string inputs without changing established setter contracts.

Summary

This PR fixes aliasing-related use-after-free behavior in several WW3D2 string setters by duplicating incoming strings before releasing existing owned storage.

  • Updates the shared aggregate and parameter setters.
  • Applies equivalent particle-emitter changes to both Generals and Generals Zero Hour.
  • Preserves existing null-input behavior and allocator pairing.

Reviews (1) · Last reviewed commit: "fix(ww3d2): Copy string setter inputs be..."

@xezon xezon changed the title fix(ww3d2): Copy string setter inputs before freeing aliased storage fix(ww3d2): Copy string setter inputs before freeing potentially aliased storage Sep 19, 2026

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Makes sense.

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing Stability Concerns stability of the runtime labels Sep 19, 2026
@xezon xezon added this to the Stability fixes milestone Sep 19, 2026
@xezon
xezon merged commit e401710 into TheSuperHackers:main Sep 20, 2026
24 checks passed
gamezerve pushed a commit to gamezerve/Reborn-Omega that referenced this pull request Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Minor Severity: Minor < Major < Critical < Blocker Stability Concerns stability of the runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants