Skip to content

GSHashTable: do not count duplicate keys when creating from an array - #74

Merged
fredkiefer merged 2 commits into
gnustep:masterfrom
DTW-Thalion:fix/gshashtable-create-dedup
Aug 29, 2026
Merged

fredkiefer merged 2 commits into
gnustep:masterfrom
DTW-Thalion:fix/gshashtable-create-dedup

Conversation

@DTW-Thalion

Copy link
Copy Markdown
Contributor

Problem

GSHashTableCreate builds the table from the input array by calling GSHashTableAddKeyValuePair and incrementing _count for every element, with no check for a key that is already present:

bucket = GSHashTableFindBucket (new, keys[idx], _kGSHashTableInsert);
GSHashTableAddKeyValuePair (new, bucket, keys[idx], values[idx]);
new->_count += 1;

For a duplicate key, GSHashTableFindBucket returns the existing bucket, but _count is still incremented, so it ends up larger than the number of stored buckets. Since GSHashTableGetCount returns _count while GSHashTableGetKeysAndValues emits one entry per occupied bucket, the two disagree:

  • CFSetGetCount / CFDictionaryGetCount over-report (e.g. CFSetCreate of {a, b, a} reports 3).
  • CFSetGetValues (and the dictionary/bag equivalents) then read past the filled buckets into uninitialised memory.

This affects CFSetCreate, CFDictionaryCreate and CFBagCreate, which all share this path.

Fix

Guard the insert with the same bucket->count <= 0 check the mutable GSHashTableAddValue path already uses, so a key that is already present is not added or counted again. Creation now behaves consistently with incremental insertion.

Tests

Tests/CFSet/create_duplicates.m and Tests/CFDictionary/create_duplicates.m create a container from an array containing a repeated key and check the count. The dictionary test fails before this change (count 2) and passes after (count 1); both pass after the fix.

GSHashTableCreate added every element of the input array and incremented
_count each time without checking whether the key was already present, so a
repeated key inflated the count past the number of stored buckets.  That made
CFSetGetCount / CFDictionaryGetCount over-report, and left CFSetGetValues
reading uninitialised slots.  Guard the insert with the same bucket->count
check the mutable GSHashTableAddValue path already uses.

Adds Tests/CFSet/create_duplicates.m and Tests/CFDictionary/create_duplicates.m.
@DTW-Thalion
DTW-Thalion force-pushed the fix/gshashtable-create-dedup branch from b026de4 to 45132b8 Compare July 24, 2026 15:11
@DTW-Thalion DTW-Thalion reopened this Jul 24, 2026
@DTW-Thalion DTW-Thalion reopened this Jul 27, 2026
@HendrikHuebner

Copy link
Copy Markdown
Contributor

Some tests failed.

Skipping duplicate keys unconditionally in GSHashTableCreate also
dropped the repeat counts a CFBag must keep, since it is a multiset.
Thread a shouldCount flag from the callers so bags still count
repeated values while sets and dictionaries store each key once.
@fredkiefer
fredkiefer merged commit 455609e into gnustep:master Aug 29, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants