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.
