Re: Change GUC hashtable to use simplehash?

On 22.01.24 03:03, John Naylor wrote:
> I wrote:
>>    fasthash_init(&hs, sizeof(Datum), kind);
>>    fasthash_accum(&hs, (char *) &value, sizeof(Datum));
>>    return fasthash_final32(&hs, 0);
> It occurred to me that it's strange to have two places that length can
> be passed. That was a side effect of the original, which used length
> to both know how many bytes to read, and to modify the internal seed.
> With the incremental API, it doesn't make sense to pass the length (or
> a dummy macro) up front -- with a compile-time fixed length, it can't
> possibly break a tie, so it's just noise.
> 
> 0001 removes the length from initialization in the incremental
> interface. The standalone functions use length directly the same as
> before, but after initialization. Thoughts?

Unrelated related issue: src/include/common/hashfn_unstable.h currently 
causes warnings from cpluspluscheck:

/tmp/cirrus-ci-build/src/include/common/hashfn_unstable.h: In function 
‘int fasthash_accum_cstring_unaligned(fasthash_state*, const char*)’:
/tmp/cirrus-ci-build/src/include/common/hashfn_unstable.h:201:20: 
warning: comparison of integer expressions of different signedness: 
‘int’ and ‘long unsigned int’ [-Wsign-compare]
    201 |   while (chunk_len < FH_SIZEOF_ACCUM && str[chunk_len] != '\0')
        |                    ^

and a few more like that.

I think it would be better to declare various int variables and 
arguments as size_t instead.  Even if you don't actually need the larger 
range, it would make it more self-documenting.