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
