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.
