refactor(wwlib): Copy CPUID registers into the CPU vendor and name strings instead of an unsigned cast by CryoTheRenegade · Pull Request #3636 · TheSuperHackers/GeneralsGameCode

CryoTheRenegade · 2026-10-30T04:58:27Z

Follow-up to #3580, #3635. cpudetect.cpp fills the CPU vendor and processor name strings by passing char array elements to CPUDetectClass::CPUID as unsigned&. Writing an unsigned through a reference into a char array is undefined behavior, the same problem as #3521.

Init_Processor_Manufacturer: receives EBX, ECX and EDX in unsigned locals and copies them into VendorID with memcpy, in the same EBX, EDX, ECX order as before.
Init_Processor_String: the three brand string calls (0x80000002 to 0x80000004) go through a new Copy_CPUID_String helper. It receives the four registers in an unsigned array and copies all 16 bytes into ProcessorString.

CPUID returns false without writing anything when the CPU has no CPUID instruction. Both sites copy only when it returns true, so the strings are left as they were in that case, the same as before.

Evidence

Disassembly: I compared cpudetect.obj against a build of main. No other function changed.
- VS 2026 (ClangCL) Release: Init_Processor_String is identical. In Init_Processor_Manufacturer, two of the three VendorID stores swapped order. Each register still goes to the same offset.
- VC6 Release: in Init_Processor_Manufacturer, two registers swapped roles. Each value still goes to the same VendorID offset. In Init_Processor_String, main inlined the three brand string CPUID calls. The branch calls CPUDetectClass::CPUID instead and stores the same four registers to the same ProcessorString offsets. The function also no longer needs an EBP frame. The fallback branch that composes the name for older CPUs makes one more call to StringClass::operator+= instead of inlining it.

AI Disclosure

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

…ssor strings Init_Processor_Manufacturer and Init_Processor_String passed character array elements as unsigned references to CPUDetectClass::CPUID, so the register values were written through an unrelated type. Receive the registers in unsigned variables and copy their bytes into the same string positions. When CPUID fails nothing is written, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector · 2026-10-30T04:58:41Z

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-30T05:00:07.610375Z We couldn’t complete this review. Comment `@codex review` to try again.	`8ff52ce`	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.

coderabbitai · 2026-10-30T04:58:53Z

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: bd8acb0b-1f49-4dea-b1bc-32ed54af4b19

Commits

Reviewing files that changed from the base of the PR and between 8ff52ce and af3541f.

Files selected for processing: 1

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

Files skipped from review as they are similar to previous changes: 1

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

Walkthrough

CPUID vendor and processor-string initialization now checks for successful CPUID calls before copying output registers into the destination buffers.

Changes

CPUID string extraction

Layer / File(s)	Summary
Checked CPUID result copying `Core/Libraries/Source/WWVegas/WWLib/cpudetect.cpp`	`Init_Processor_Manufacturer` copies vendor registers in EBX, EDX, ECX order after a successful CPUID call. `Copy_CPUID_String` copies successful CPUID results to the processor string. `Init_Processor_String` uses the helper for leaves `0x80000002` through `0x80000004`.

Priority: Low

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

Change: Refactor

Suggested reviewers: xezon

Merge Risk: Minimal · up to af354

No merge-blocking issue was found in the CPUID string initialization changes. Mergeable after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: the CPUID register copies in cpudetect.cpp now write into the CPU vendor and name strings through copies instead of an unsigned cast. The raw summary confirms Init_Pro…
Description check		The description matches the changeset. It explains that cpudetect.cpp passed char array elements as unsigned& to CPUDetectClass::CPUID, and that the change adds memcpy copies to Init_Processor_Manufac…
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.

Full details: Title check

Explanation

The title names the main change: the CPUID register copies in cpudetect.cpp now write into the CPU vendor and name strings through copies instead of an unsigned cast. The raw summary confirms Init_Processor_Manufacturer and Init_Processor_String were changed to do this, with a Copy_CPUID_String helper added. The title is specific and matches the changeset. It uses the conventional refactor(scope) prefix and is a little long at 102 characters, which does not change the verdict.

Full details: Description check

Explanation

The description matches the changeset. It explains that cpudetect.cpp passed char array elements as unsigned& to CPUDetectClass::CPUID, and that the change adds memcpy copies to Init_Processor_Manufacturer and a Copy_CPUID_String helper to Init_Processor_String. It also covers the CPUID-unavailable path and gives disassembly evidence. The description is detailed and on topic.

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-30T05:01:22Z

Confidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Replaces writes through cast references with copies from unsigned locals into the CPU vendor and processor-name strings.

The CPU detector copies register values into the vendor ID string.
The CPU detector copies register values into the processor name string.

_{Reviews (2) · Last reviewed commit: "style(wwlib): Spell out unsigned int in ..." · Reviewed by Greptile}

Skyaero42 · 2026-10-30T07:55:40Z

+	if (CPUID(max_cpuid, ebx, ecx, edx, 0))
+	{
+		// The vendor string is stored in EBX, EDX, ECX order
+		memcpy(&VendorID[0], &ebx, sizeof(ebx));


is unsigned guaranteed to be 4 bytes?

on an x86 system it is, unsigned is just an alias for unsigned int.

i prefer to state the type as well instead of leaving it to be inffered as int

Changed to unsigned int for clarity

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CryoTheRenegade and others added 2 commits October 23, 2026 16:56

style(wwlib): Use Allman braces in the CPUID string copies

8ff52ce

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Skyaero42 reviewed Oct 30, 2026

View reviewed changes

style(wwlib): Spell out unsigned int in the CPUID string copies

af3541f

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Navigation Menu

Repository navigation

refactor(wwlib): Copy CPUID registers into the CPU vendor and name strings instead of an unsigned cast - #3636

refactor(wwlib): Copy CPUID registers into the CPU vendor and name strings instead of an unsigned cast#3636
CryoTheRenegade wants to merge 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:refactor/cpuid-strings

CryoTheRenegade commented Oct 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 30, 2026 •

edited

Loading

Summary

Uh oh!

Skyaero42 Oct 30, 2026

Uh oh!

Mauller Oct 30, 2026

Uh oh!

CryoTheRenegade Oct 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

CryoTheRenegade commented Oct 30, 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 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

coderabbitai Bot commented Oct 30, 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 30, 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 Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

CryoTheRenegade Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CryoTheRenegade commented Oct 30, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading

coderabbitai Bot commented Oct 30, 2026 •

edited

Loading

greptile-apps Bot commented Oct 30, 2026 •

edited

Loading