Re: define pg_structiszero(addr, s, r)

On Tue, Nov 12, 2024 at 06:09:04AM +0000, Bertrand Drouvot wrote:
> I think that the 64b len check done in v11 is mandatory for safety reasons.
> 
> The loop above reads 64 bytes at once, so would read beyond the memory area bounds
> if len < 64: That could cause crash or read invalid data.

Sorry, I was not following your argument.  You're right that we need
something else here.  However..

+	/*
+	 * For len < 64, compare byte per byte to ensure we'll not read beyond the
+	 * memory area.
+	 */
+	if (len < sizeof(size_t) * 8)
+	{
+		while (p < end)
+		{
+			if (*p++ != 0)
+				return false;
+		}
+		return true;
+	}
+
+	/* Compare bytes until the pointer "p" is aligned */
+	while (((uintptr_t) p & (sizeof(size_t) - 1)) != 0)
+	{
+		if (p == end)
+			return true;
+
+		if (*p++ != 0)
+			return false;
+	}
+

Still, this is not optimal, based on what's been discussed upthread.
The byte-per-byte check is more expensive than the size_t check, so
shouldn't you make sure that you stack some size_t checks if dealing
with something smaller than 64 bytes?  That would be a bit more
complex, sure, but if you leave that within the block doing "len <
sizeof(size_t) * 8", perhaps that's OK..  Or just do what you
mentioned upthread with a second macro for sizes <= 64.  You'd need
three steps in this first block rather than one:
- byte-per-byte, up to aligned location.
- size_t loop.
- Again byte-per-byte, until the end,
--
Michael