On 01.09.25 05:25, Michael Paquier wrote:
> So, taking a step back, I don't know what would be a good fit for
> these duplicates of the "next power" routines upper-bounded on input
> when attached to pg_bitutils.h. However, I do see that we can get rid
> of pg_log2() and dynahash.h with a consistent interface in
> pg_bitutils.h, by reducing my proposal to the introduction of
> pg_ceil_log2_32_bound() and pg_ceil_log2_64_bound().
>
> At the end, next_pow2_int64() and next_pow2_int() are a lesser deal to
> me, being static to dynahash.c. With that in mind, I am finishing
> with the attached. Less ambitious, still it's a nice cleanup IMO.
pg_bitutils.h is aligned with standard compiler intrinsics and in the
long term C23 <stdbit.h>, so we shouldn't add our own custom stuff in
there without considering that bigger picture.
I would agree with what I think you're saying, we can keep these custom
variants with a particular error-checking behavior local to dynahash.c.
Maybe a comment or two to explain this more clearly would be good.
Taking a look at your previous patch with the changes from long to
int64, I think there is something that still doesn't fit.
For example, taking a look at the callers of hash_estimate_size(int64,
Size), they pass either int as the first argument, or in a few cases
long. Looking around inside dynahash.c, do any of these places actually
need the int64 range? These are all just counters, the memory sizes use
Size correctly it seems. Do we want to support more than INT_MAX
elements? I wonder whether the right solution would be to turn the long
uses into int instead. Then you also don't need to deal with two
next_pow2* variants.
