On 01.10.26 22:39, Andres Freund wrote:
> I don't like that this is adding a large number of unconstify()s. The only
> thing that makes it a bit awkward to make endptr const itself is
> endptr_p. It'd be tempting to just remove it, most callers don't even use it -
> but unfortunately it looks like single_decode() does.
>
> It seems like it'd be less ugly to just unconstify the two places that
> actually need it (the call to strtod() and the assignment to *endptr_p).
Yeah, done that way in the new version.
>> From bb7d887646ec7f1a83d1a5f9e370861a82c86ff8 Mon Sep 17 00:00:00 2001
>> From: Peter Eisentraut <peter@eisentraut.org>
>> Date: Tue, 15 Sep 2026 08:43:09 +0200
>> Subject: [PATCH v2 7/8] Additional unvolatize uses
>>
>> Add some uses of unvolatize to silence warnings that would be
>> triggered by -Wcast-qual.
>>
>> Note that the MemSet() calls would be normal "discards qualifier"
>> warnings if memset() (or another function with a prototype, not a
>> macro) were used.
>
> ISTM just about all these volatiles are just pointless magic-wand
> volatiles. volatile doesn't fix memory ordering (except on msvc), assigning
> volatile to entire struct is bogus hokus pokus.
>
> The fact that we have to unvolatize them shows that we are fundamentally not
> relying on the guarantees of volatile, the invoked functions don't know about
> the volatile!
I think arguments can be made that some of these volatile qualification
are unnecessary. But for example for signal handling, using volatile
sig_atomic_t is kind of idiomatic, and for the pgstat stuff, a change
would probably call for a broad cleanup of volatile in that module.
That might be too large of a detour?
Attached is a version that rewrites the code lightly to avoid the
"unvolatize" casts honestly but keeps the qualifiers themselves.
