Subject: Re: bug#56413: [PATCH 1/1] scm_i_utf8_string_hash: compute u8 chars not bytes

From: Ludovic Courtès <[email-masked]>
Date: Sat, 05 Nov 2022 23:18:26 +0100

Hi,

Rob Browning <[email-masked]> skribis:

> Noticed while investigating a migration to utf-8 strings.  After making
> changes that routed non-ascii symbol hashing through this function,
> encoding-iso88597.test began intermittently failing because it would
> traverse trailing garbage when u8_strnlen reported 8 chars instead of 4.
>
> Change the scm_i_str2symbol internal hash type to unsigned long to
> explicitly match the hashing result type.

Oh, good catch.

For the final patch please add a ChangeLog-style entry.

> +  // Make sure a utf-8 symbol has the expected hash.  In addition to
> +  // catching algorithmic regressions, this would have caught a
> +  // long-standing buffer overflow.
> +
> +  // περί
> +  char about_u8[] = {0xce, 0xa0, 0xce, 0xb5, 0xcf, 0x81, 0xce, 0xaf, 0};
> +  SCM sym = scm_from_utf8_symbol (about_u8);
> +
> +  const unsigned long expect = 4029223418961680680;
> +  const unsigned long actual = scm_to_ulong (scm_symbol_hash (sym));

Is this a documented example of Jenkins?  Or did you use a reference
implementation?

> Hmm.  I suppose the current test could be handled on the scheme side
> instead.  (I'd started off attempting some more direct, elaborate tests
> that didn't pan out.)  Happy to rework that if desired.

Yes, it may be nicer to have it in ‘test-suite/tests/hash.test’.

AFAICS this will only change the hash of UTF-8 symbols and won’t have
any effect on the output of ‘string-hash’, right?  If not that would be
an incompatibility.

Thanks and sorry for the delay!

Ludo’.