Skip to content

Commit

Permalink
[libromdata] AmiiboData: Restore the full binary search for the chara…
Browse files Browse the repository at this point in the history
…cter variant ID check.

The issue was that we weren't immediately returning `false` if the
character ID was *greater* than the expected character ID. This caused
a broken ordering, which triggered the libstdc++ assertion in debug mode.

Hence, if `key1.char_id > key2.char_id`, we shouldn't even bother checing
the character variant ID, since we know key2 will effectively be "greater
than" key1 regardless.

Remember: C++ binary search functions return true if `a < b`, whereas
C binary search functions return an strcmp()-like value. (-1, 0, 1)

RomHeaderTest now passes all (re-generated) Amiibo tests with no errors.
  • Loading branch information
GerbilSoft committed Nov 17, 2024
1 parent dfd5356 commit 196dfe2
Showing 1 changed file with 14 additions and 19 deletions.
33 changes: 14 additions & 19 deletions src/libromdata/data/AmiiboData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -639,31 +639,26 @@ const char *AmiiboData::lookup_char_name(uint32_t char_id) const
const uint8_t variant_id = (char_id >> 8) & 0xFF;
const CharVariantTableEntry key = {cv_char_id, variant_id, 0, 0};

// Find the character ID entries first.
// NOTE: Checking both char ID and variant ID in the lambda function
// is causing an assertion when compiled with _GLIBCXX_DEBUG.
const CharVariantTableEntry *const pCharVarTblEnd = d->pCharVarTbl + d->charVarTbl_count;
auto pCVTEntry = std::lower_bound(d->pCharVarTbl, pCharVarTblEnd, key,
[](const CharVariantTableEntry &key1, const CharVariantTableEntry &key2) noexcept -> bool {
return (le16_to_cpu(key1.char_id) < key2.char_id);
});
const uint16_t key1_char_id = le16_to_cpu(key1.char_id);

bool found = false;
if (pCVTEntry != pCharVarTblEnd && le16_to_cpu(pCVTEntry->char_id) == cv_char_id) {
// Find a matching character variant ID.
for (; pCVTEntry < pCharVarTblEnd; pCVTEntry++) {
if (le16_to_cpu(pCVTEntry->char_id) != cv_char_id) {
// All variant entries checked for this character.
break;
} else if (pCVTEntry->var_id == variant_id) {
// Found a matching variant.
found = true;
break;
// Compare the character ID first.
if (key1_char_id < key2.char_id) {
return true;
} else if (key1_char_id > key2.char_id) {
return false;
}
}
}

if (found) {
// Compare the variant ID.
return (key1.var_id < key2.var_id);
});

if (pCVTEntry != pCharVarTblEnd &&
le16_to_cpu(pCVTEntry->char_id) == cv_char_id &&
pCVTEntry->var_id == variant_id)
{
// Character variant ID found.
name = d->strTbl_lookup(le32_to_cpu(pCVTEntry->name));
} else {
Expand Down

0 comments on commit 196dfe2

Please sign in to comment.