Skip to content

fix(hash): Fix initialization of HashTableIteratorClass and make it work with an empty HashTableClass - #3284

Open
xezon wants to merge 1 commit into
TheSuperHackers:mainfrom
xezon:xezon/fix-hashtableiterator-init
Open

fix(hash): Fix initialization of HashTableIteratorClass and make it work with an empty HashTableClass#3284
xezon wants to merge 1 commit into
TheSuperHackers:mainfrom
xezon:xezon/fix-hashtableiterator-init

Conversation

@xezon

@xezon xezon commented Sep 13, 2026

Copy link
Copy Markdown

This change fixes the missing initialization in the HashTableIteratorClass constructor.

This is consistent with other iterator constructors:

GenericMultiListIterator(GenericMultiListClass *list)	{ assert(list); First(list); }
GridListIterator(CullableClass * head)		{ First(head); }

This fix looks to be inconsequential for the game runtime.

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing labels Sep 13, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Initialize HashTableIteratorClass on construction

🐞 Bug fix 🕐 Less than 5 minutes

Grey Divider

AI Description

• Initializes hash-table iterators immediately during construction.
• Positions new iterators at the table's first available entry.
Diagram

graph TD
  A["Iterator constructor"] -->|calls| B["First"] -->|initializes| C["Iteration state"]
Loading
High-Level Assessment

Calling the existing First() method from the constructor is the most direct approach because it establishes valid iterator state immediately and matches other iterator constructors. Lazy initialization would complicate every consumer operation without providing a practical benefit.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1
hash.hInitialize hash-table iterator state in its constructor +1/-1

Initialize hash-table iterator state in its constructor

• The 'HashTableIteratorClass' constructor now calls 'First()' after binding the table reference. This initializes traversal state and positions the iterator at the first available entry.

Core/Libraries/Source/WWVegas/WWLib/hash.h

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 13, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Empty hash tables can crash during iterator construction 🐞 Bug ☼ Reliability
Description
HashTableIteratorClass now calls First() from its constructor, and First() immediately reads
Table.HashTable[0]. A zero-sized HashTableClass passes its current power-of-two assertion and
allocates a zero-length array, so merely constructing an iterator for such a table performs an
out-of-bounds read before callers can check or avoid iteration.
Code

Core/Libraries/Source/WWVegas/WWLib/hash.h[96]

+	HashTableIteratorClass( HashTableClass & table ) : Table( table ) { First(); }
Evidence
The table constructor only asserts (HashTableSize & (HashTableSize-1)) == 0, which evaluates true
for zero, then allocates HashTableSize buckets. The changed iterator constructor now invokes
First(), while First() unconditionally reads bucket zero before checking whether the table has
any buckets.

Core/Libraries/Source/WWVegas/WWLib/hash.cpp[45-53]
Core/Libraries/Source/WWVegas/WWLib/hash.cpp[131-136]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`HashTableIteratorClass` eagerly calls `First()`, whose first operation reads bucket zero. `HashTableClass` currently permits a size of zero because its power-of-two assertion does not reject it, so constructing an iterator for a zero-sized table can read out of bounds.
## Fix Focus Areas
- Core/Libraries/Source/WWVegas/WWLib/hash.h[96-96]
- Core/Libraries/Source/WWVegas/WWLib/hash.cpp[45-53]
- Core/Libraries/Source/WWVegas/WWLib/hash.cpp[131-136]
## Recommended Fix
Either reject zero-sized tables in `HashTableClass` construction and ensure callers provide a positive power-of-two size, or make `First()` detect `HashTableSize == 0` before accessing `HashTable[0]` and leave the iterator in the done state. Add a regression test covering iterator construction with an empty-sized table if zero-sized tables are intended to be supported.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Core/Libraries/Source/WWVegas/WWLib/hash.h
@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR initializes HashTableIteratorClass during construction and corrects bucket-index advancement.

  • Calls First() from the iterator constructor.
  • Safely supports empty hash tables without accessing bucket zero.
  • Preserves traversal across the first and final buckets without skipping entries.

Confidence Score: 5/5

The pull request appears safe to merge with no outstanding correctness or repository-rule issues.

The iterator now initializes its current state, checks the table boundary before bucket access, and advances its bucket index exactly once after each valid lookup.

Important Files Changed

Filename Overview
Core/Libraries/Source/WWVegas/WWLib/hash.cpp Adjusts iterator initialization and bucket advancement to handle empty tables and traverse all buckets safely.
Core/Libraries/Source/WWVegas/WWLib/hash.h Initializes iterator state by calling First() from the constructor.

Reviews (2): Last reviewed commit: "fix(hash): Fix initialization of HashTab..." | Re-trigger Greptile

@xezon
xezon force-pushed the xezon/fix-hashtableiterator-init branch from 57f89ff to 3d9916c Compare September 13, 2026 06:57
@xezon xezon changed the title fix(hash): Fix HashTableIteratorClass constructor fix(hash): Fix initialization of HashTableIteratorClass and make it work with an empty HashTableClass Sep 13, 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.

1 participant