Re: define pg_structiszero(addr, s, r)

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Ranier Vilela <ranier.vf@gmail.com>, David Rowley <dgrowleyml@gmail.com>, Peter Smith <smithpb2250@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@lists.postgresql.org
Date: 2024-11-15T06:46:52Z
Lists: pgsql-hackers

Attachments

Hi,

On Fri, Nov 15, 2024 at 09:30:25AM +0900, Michael Paquier wrote:
> On Thu, Nov 14, 2024 at 12:33:20PM +0000, Bertrand Drouvot wrote:
> 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).

Yeah, thanks for the testing!

> > 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.

Makes sense even if that looks "more difficult" to read.

> 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.

Thanks! Applied on v13 attached, except for things like:

"
-       /* Compare bytes until the pointer "p" is aligned */
+       /* Compare bytes until the pointer "p" is aligned. */
"

which is adding a "." at the end of single line comments (as the few already
part of this file don't do so).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Commits

  1. Use pg_memory_is_all_zeros() in PageIsVerifiedExtended()

  2. Optimize pg_memory_is_all_zeros() in memutils.h

  3. Remove use of pg_memory_is_all_zeros() in bufpage.c

  4. Add pg_memory_is_all_zeros() in memutils.h