fix(wwlib): Load the StringClass hash word with memcpy instead of an unsigned int cast by CryoTheRenegade · Pull Request #3630 · TheSuperHackers/GeneralsGameCode

CryoTheRenegade · 2026-10-29T02:33:03Z

Follow-up to #3580, #3605, #3617 and #3628. HashTemplateKeyClass<StringClass>::Get_Hash_Value reads four characters of the string through an unsigned int pointer. Reading a char buffer as an unsigned int is undefined behavior, the same problem as #3521, and the address isn't necessarily 4-byte aligned.

The function now copies the four bytes into a uint32_t with memcpy, so the size of the load no longer depends on the size of unsigned int. The offset, the length check and the byte order are unchanged, so every string gets the same hash as before.

-	unsigned int hval = *((const unsigned int*)(buffer+len-8));
+	uint32_t hval;
+	memcpy(&hval, buffer+len-8, sizeof(hval));
 	hval = hval + (hval>>5) + (hval>>10) + (hval >> 20);

hashtemplate.h now includes <string.h> for memcpy directly. Its existing includes only bring it in through the VC6 branch of bit_adapter.h. uint32_t already comes in through stdint_adapter.h in the precompiled header.

This hash is used for the texture, hierarchy tree and definition lookup tables.

Evidence

Disassembly: I compared every rebuilt object against a build of main.
- VS 2026 Release: no function changed. The only differences are label and section numbering.
- VC6 Release: the hash code is the same. Apart from label renumbering from the new include, the only difference is the __TIME__ string in WinMain.
- uint32_t instead of unsigned int: the disassembly of assetmgr.cpp.obj, which uses this hash, is identical before and after on VS 2026 Release and VC6 Release.

AI Disclosure

Claude Opus 5.5 was used to aid in implementation and to compare the disassembly.

HashTemplateKeyClass<StringClass>::Get_Hash_Value read four characters of the string as an unsigned int through a pointer cast. Copy them into an unsigned int with memcpy instead. The offset, length check and byte order are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector · 2026-10-29T02:33:11Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-29T02:33:07.896440Z We couldn’t complete this review. Comment `@codex review` to try again.	`abba23a`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-10-29T02:33:13Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

coderabbitai · 2026-10-29T02:33:24Z

No actionable comments were generated in the recent review.

Recent 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: f0c90473-fc3d-4855-af23-5a7a2cdf08cc

Commits

Reviewing files that changed from the base of the PR and between abba23a and 3490a9b.

Files selected for processing: 1

Core/Libraries/Source/WWVegas/WWLib/hashtemplate.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.

Walkthrough

The StringClass hash specialization now uses memcpy to copy four bytes into a uint32_t instead of reading them through an unsigned int pointer. The file includes <string.h>.

Changes

String hash

Layer / File(s)	Summary
String hash input read `Core/Libraries/Source/WWVegas/WWLib/hashtemplate.h`	The specialization copies four bytes into a `uint32_t` with `memcpy`. The hash calculation remains unchanged.

Priority: Low

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

Change: Refactor

Possibly related PRs

TheSuperHackers/GeneralsGameCode#3532: Replaces pointer-based bit reinterpretation in several locations, including another hash specialization in hashtemplate.h.
TheSuperHackers/GeneralsGameCode#3580: Replaces unsafe type reinterpretation in vector and color code.

Suggested reviewers: xezon

Merge Risk: Minimal · up to 3490a

The change removes unsafe memory access while preserving hash behavior, with no merge-blocking risk identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly summarizes the main change: replacing the unsigned int pointer read with memcpy for StringClass hash loading.
Description check		The description directly explains the undefined behavior, the memcpy-based fix, compatibility expectations, included headers, affected lookup tables, and verification evidence.
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

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

greptile-apps · 2026-10-29T02:34:33Z

Confidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR replaces the cast-based string hash read with memcpy and adds <string.h>. The latest change makes the copied word explicitly 32 bits with uint32_t.

String keys keep their hash while the four-byte read uses memcpy.

_{Reviews (2) · Last reviewed commit: "fix(wwlib): Use uint32_t for the StringC..." · Reviewed by Greptile}

Skyaero42

I think this is a fix rather than a refactor.

Skyaero42 · 2026-10-29T13:42:22Z

 	}
-	unsigned int hval = *((const unsigned int*)(buffer+len-8));
+	unsigned int hval;
+	memcpy(&hval, buffer+len-8, sizeof(hval));


I believe you should not assume that an unsigned int always have 4 bytes. Instead uint32_t should be used instead, which guarantees four bytes.

changed, thanks!

The hash word is four bytes copied from the string, so declare it as uint32_t rather than relying on unsigned int being four bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Skyaero42 reviewed Oct 29, 2026

View reviewed changes

CryoTheRenegade changed the title ~~refactor(wwlib): Load the StringClass hash word with memcpy instead of an unsigned int cast~~ fix(wwlib): Load the StringClass hash word with memcpy instead of an unsigned int cast Oct 29, 2026

fix(wwlib): Use uint32_t for the StringClass hash word

3490a9b

The hash word is four bytes copied from the string, so declare it as uint32_t rather than relying on unsigned int being four bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Navigation Menu

Repository navigation

fix(wwlib): Load the StringClass hash word with memcpy instead of an unsigned int cast - #3630

fix(wwlib): Load the StringClass hash word with memcpy instead of an unsigned int cast#3630
CryoTheRenegade wants to merge 2 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:refactor/string-hash-load

CryoTheRenegade commented Oct 29, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Uh oh!

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 29, 2026 •

edited

Loading

Summary

Uh oh!

Skyaero42 left a comment

Uh oh!

Skyaero42 Oct 29, 2026

Uh oh!

CryoTheRenegade Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

CryoTheRenegade commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Evidence

AI Disclosure

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Uh oh!

coderabbitai Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

greptile-apps Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Uh oh!

Skyaero42 left a comment

Choose a reason for hiding this comment

Uh oh!

Skyaero42 Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

CryoTheRenegade Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

CryoTheRenegade commented Oct 29, 2026 •

edited

Loading

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading

greptile-apps Bot commented Oct 29, 2026 •

edited

Loading