Re: define pg_structiszero(addr, s, r)

On Thu, Nov 14, 2024 at 12:33:20PM +0000, Bertrand Drouvot wrote:
> Case 2 should be read as "in the 4-31" bytes range on 32-bit system as all
> comparisons are done in size_t.

I'd suggest to use a -m32 in your gcc switches, when it comes to
tests, but you already know that.. Anyway, as you say, the
portability of v12 is OK even for sizeof(size_t) == 4 because we don't
rely on any hardcoded values, and this patch does what it should in
this case (double-checked myself manually for the three cases with
-m32).

> What would be unsafe on 32-bit would be to read up to 32 bytes while len < 32
> and that can not happen.
>
> As mentioned up-thread the comments are wrong on 32-bit, indeed they must be read
> as:
>
> Case 1: len < 4 bytes
> Case 2: len in the 4-31 bytes range
> Case 3: len >= 32 bytes

This part could be indeed better than what's proposed in v12, so I
would recommend to use sizeof(size_t) a bit more consistently rather
than have the reader guess that. Note that some parts of v12-0001 use
sizeof(size_t) in the comments, which makes things inconsistent.

Some comments feel duplicated, as well, like the "no risk" mentions,
which are clear enough based on the description and the limitations of
the previous cases. I'd like to suggest a few tweaks, making the
comments more flexible. See 0003 that applies on top of your latest
patch set, reattaching v12 again.
--
Michael