Skip to content

fix(ini): Prevent using unitialized stack variable in INI::parseBitString8() - #3307

Merged
xezon merged 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:bugfix/clang-tidy-ini-byte-bitstring
Sep 17, 2026
Merged

xezon merged 1 commit into
TheSuperHackers:mainfrom
CryoTheRenegade:bugfix/clang-tidy-ini-byte-bitstring

Conversation

@CryoTheRenegade

Copy link
Copy Markdown

Initialize parseBitString8's temporary from the existing stored byte before calling parseBitString32.

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: 4fb21249-ead5-4d8e-b38a-56a2d2d26449

📥 Commits

Reviewing files that changed from the base of the PR and between 039bd17 and bc737a0.

📒 Files selected for processing (1)
  • Core/GameEngine/Source/Common/INI/INI.cpp

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


Walkthrough

parseBitString8 now initializes its temporary value from the existing stored byte before applying bit-string operations. This replaces the previous uninitialized starting state.

Changes

INI bit-string parsing

Layer / File(s) Summary
Preserve the existing byte during parsing
Core/GameEngine/Source/Common/INI/INI.cpp
parseBitString8 starts bit-string modifications from the existing byte value. Subsequent + and - operations now use that initialized state.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to bc737

This change preserves the existing byte when applying INI bit-string operations, and no material merge-blocking risk remains.

🚥 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.
Description check ✅ Passed The description clearly explains the initialization change in INI::parseBitString8 and identifies clang-tidy as the source of the finding.
Title check ✅ Passed The title clearly identifies the fix for the uninitialized stack variable in INI::parseBitString8.

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, with no actionable correctness, security, or repository-rule violations identified.

Summary

This PR corrects 8-bit INI bit-string parsing by initializing its 32-bit temporary from the currently stored byte.

  • Preserves the existing flags when processing additive or subtractive tokens.
  • Retains replacement behavior for ordinary tokens because the delegated parser clears the temporary itself.
  • Keeps the existing byte-range validation unchanged.

Reviews (1) · Last reviewed commit: "fix(INI): Initialize the temporary from ..."

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing labels Sep 17, 2026
@xezon xezon changed the title fix(INI): Initialize the temporary from the existing byte value fix(ini): Prevent using unitialized stack variable in INI::parseBitString8() Sep 17, 2026
@xezon
xezon merged commit 8605738 into TheSuperHackers:main Sep 17, 2026
24 checks passed
gamezerve pushed a commit to gamezerve/Reborn-Omega that referenced this pull request Sep 17, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants