Date: Wed, 18 Sep 2024 18:57:39 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: define pg_structiszero(addr, s, r)
Message-ID: <ZusioyiXTyKpOP/2@ip-10-97-1-34.eu-west-3.compute.internal>
References: <ZupUDDyf1hHI4ibn@ip-10-97-1-34.eu-west-3.compute.internal>
 <3023d29e-0fe7-4bec-9028-7f3c0102e437@eisentraut.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3023d29e-0fe7-4bec-9028-7f3c0102e437@eisentraut.org>
Archived-At: <https://www.postgresql.org/message-id/ZusioyiXTyKpOP/2%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote:
> On 18.09.24 06:16, Bertrand Drouvot wrote:
> > +#define pg_structiszero(addr, s, r)									\
> > +	do {															\
> > +		/* We assume this initializes to zeroes */					\
> > +		static const s all_zeroes;									\
> > +		r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0);	\
> > +	} while (0)
>

Thanks for the feedback.
 
> This assumption is kind of the problem, isn't it?  Because, you can't assume
> that.  And the existing code is arguably kind of wrong.  But moreover, this
> macro also assumes that the "addr" argument has no random padding bits.
> 
> In the existing code, you can maybe make a local analysis that the code is
> working correctly, although I'm not actually sure.

I think it is but will give it a closer look.

>  But if you are
> repackaging this as a general macro under a general-sounding name, then the
> requirements should be more stringent.

Agree. That said in v2 ([1]), it has been renamed to pgstat_entry_all_zeros().

I think that I will:

1/ take a closer look regarding the existing assumption
2/ if 1/ outcome is fine, then add more detailed comments around
pgstat_entry_all_zeros() to make sure it's not used outside of the existing
context 

Sounds good to you?

[1]: https://www.postgresql.org/message-id/ZuqHLCdZXtEsbyb/%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

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