On Fri, 10 Oct 2025 at 11:30, Michael Paquier <michael@paquier.xyz> wrote:
> Thanks for double-checking. Applied after running an indent.
I was working on test_bitmapset.c to add some tests for a new
bitmapset function. I noticed a few weird things.
1. test_random_operations() is coded to use GetCurrentTimestamp() as a
seed when the given seed is <= 0. Of course, it'll be a while before
the return value of that wraps beyond 2^63 (292250 years), but I still
can't help but think that NULL is a better value to use to have the
seed auto-generate.
2. Doing #1 means the function can't be STRICT. I do think it's wrong
that the function is marked as strict. That's normally reserved for
functions that we needn't call because NULL input(s) yield a NULL
output. That's not the case for this function.
3. There's no CHECK_FOR_INTERRUPTS() in test_random_operations(). If
someone uses a large num_ops, there's no way to cancel the query.
4. If there happened to be some rare bug in bitmapset.c that
test_random_operations() we might struggle to find it again, as we
don't report which seed we used in the ERROR message.
5. Couple of minor cosmetics; header order, whilespace.
I felt it was worth fixing these now as the function I plan to add
there does #1, #2, #3 and #4. If I add the new function for v20, the
discrepancy seems questionable.
David
