| From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: fix more casting away of qualifiers |
| Date: | 2026-10-07 17:58:35 |
| Message-ID: | 37dca86c-92b5-4361-ba64-d6611132274a@eisentraut.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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(at)eisentraut(dot)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.
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Change-float-4-8-in_internal-take-const-char-inpu.patch | text/plain | 4.6 KB |
| v3-0002-Fix-casting-away-of-volatile-qualifier.patch | text/plain | 4.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-10-07 18:31:04 | Re: [PATCH] Unify duplicate-option handling across utility commands |
| Previous Message | Alexandre Felipe | 2026-10-07 17:52:18 | Re: LWLock granular partition lock memory layout |