Skip to content

fix(similarity): skip empty fingerprint sort - #1368

Open
WarGloom wants to merge 1 commit into
DeusData:mainfrom
WarGloom:agent/fix-empty-similarity-sort
Open

fix(similarity): skip empty fingerprint sort#1368
WarGloom wants to merge 1 commit into
DeusData:mainfrom
WarGloom:agent/fix-empty-similarity-sort

Conversation

@WarGloom

Copy link
Copy Markdown
Contributor

What changed

collect_fp_entries() now calls qsort only when at least two fingerprint
entries were collected.

Why

The empty path leaves entries == NULL. Calling qsort(NULL, 0, ...) relies on
library behavior outside the portable array-pointer contract, while the
one-entry path needs no sorting at all. Guarding with count > 1 avoids both
unnecessary cases without changing deterministic ordering.

Fixes #1367.

Validation

  • scripts/build.sh — passed
  • scripts/test.sh --suites simhash — passed, 24 sanitizer-backed tests
  • git diff --check — passed

Checklist

  • Commit is signed off (DCO)
  • Change is focused on one tracked bug
  • Existing focused tests pass

Signed-off-by: wargloom <wargloom@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you for this — and doubly so for reporting the issue and bringing the fix.

This is exactly the shape a fix should have: one file, one guard, and a comment that explains the why rather than restating the code. The distinction you drew in #1367 is the right one — zero elements suppress comparator calls, but that has never made a null base a valid array pointer, and count > 1 also skips the one-entry sort that could never have changed anything.

I checked the caller before merging: entry_count < MIN_FP_ENTRIES returns early via free(entries), and free(NULL) is well-defined, so the empty path stays correct end to end. Determinism at two or more entries is untouched.

Merging. Thanks again for the care you put into both the report and the patch.

@DeusData

Copy link
Copy Markdown
Owner

One thing before I merge: this is still marked as a draft, so I have held off rather than promote it myself.

If that is just left over from opening it, mark it ready (or say the word here) and it goes in as-is — the review above is complete and CI is 28/28 green. If instead you are still polishing something, take the time you need; there is no rush from our side and I would rather merge the version you consider finished.

@WarGloom
WarGloom marked this pull request as ready for review July 31, 2026 12:00
@WarGloom
WarGloom requested a review from DeusData as a code owner July 31, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pass_similarity calls qsort with a null base for an empty fingerprint set

2 participants