On 24.02.26 19:16, Bertrand Drouvot wrote:
On Tue, Feb 24, 2026 at 11:19:50AM -0600, Nathan Bossart wrote:
On Tue, Feb 24, 2026 at 05:08:09PM +0000, Bertrand Drouvot wrote:
This patch makes use of unvolatize() in vac_truncate_clog().

Note that it does not remove the warning but moves it to c.h (where unvolatize()
is defined) but that's consistent with what 481018f2804 did too.

Why is this preferable to marking the function parameter as volatile

I think that that would sound misleading for the other callers that don't really
need the volatile qualification.

or  removing the volatile qualifier from the variable?

That looks mandatory according to 2d2e40e3bef.

Arguably, putting the volatile qualifier on the whole dbform is broader than required. So you could imagine writing it something like this instead:

    FormData_pg_database *dbform = (Form_pg_database) GETSTRUCT(tuple);
    volatile TransactionId *datfrozenxid_p;
    volatile TransactionId *datminmxid_p;

    *datfrozenxid_p = dbform->datfrozenxid;
    *datminmxid_p = dbform->datminmxid;

Alternatively, we should document why applying unvolatize() is acceptable, like

    /* ... unvolatize() is ok because datconnlimit is not concurrently
       updated (see above) ...

Or just write it directly like

    if (dbform->datconnlimit == DATCONNLIMIT_INVALID_DB)

This violates the abstraction layer, but the variant with the comment kind of does too.



Reply via email to