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 <[email protected]>
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.
From 8b035adde61574bd8a19703cce3a7a510ad9936a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 7 Oct 2026 08:21:37 +0200
Subject: [PATCH v3 1/2] Change float{4,8}in_internal take const char * input
argument
This makes the function signature match strtof()/strtod().
A caller then doesn't have to cast away a const anymore.
Reviewed-by: Rui Zhao <[email protected]>
Discussion:
https://www.postgresql.org/message-id/c76b72dd-5736-453a-b698-6968d42e4768%40eisentraut.org
---
contrib/postgres_fdw/postgres_fdw.c | 2 +-
src/backend/utils/adt/float.c | 19 ++++++++-----------
src/include/utils/float.h | 4 ++--
3 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index 4ca061c9e0b..6757ee923cb 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6476,7 +6476,7 @@ set_float_arg(NullableDatum *arg, const char *s)
{
if (s)
{
- float4 val = float4in_internal((char *) s, NULL,
"float", s, NULL);
+ float4 val = float4in_internal(s, NULL, "float", s,
NULL);
arg->value = Float4GetDatum(val);
arg->isnull = false;
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index fd7a6587132..38ef6daf290 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -221,12 +221,12 @@ float4in(PG_FUNCTION_ARGS)
* comments also apply here, except regarding use in geometric types.
*/
float4
-float4in_internal(char *num, char **endptr_p,
+float4in_internal(const char *num, char **endptr_p,
const char *type_name, const char
*orig_string,
struct Node *escontext)
{
float val;
- char *endptr;
+ const char *endptr;
/*
* endptr points to the first character _after_ the sequence we
recognized
@@ -249,7 +249,7 @@ float4in_internal(char *num, char **endptr_p,
type_name, orig_string)));
errno = 0;
- val = strtof(num, &endptr);
+ val = strtof(num, unconstify(char **, &endptr));
/* did we not see anything that looks like a double? */
if (endptr == num || errno != 0)
@@ -342,7 +342,7 @@ float4in_internal(char *num, char **endptr_p,
/* report stopping point if wanted, else complain if not end of string
*/
if (endptr_p)
- *endptr_p = endptr;
+ *endptr_p = unconstify(char *, endptr);
else if (*endptr != '\0')
ereturn(escontext, 0,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
@@ -428,17 +428,14 @@ float8in(PG_FUNCTION_ARGS)
* If escontext points to an ErrorSaveContext node, that is filled instead
* of throwing an error; the caller must check SOFT_ERROR_OCCURRED()
* to detect errors.
- *
- * "num" could validly be declared "const char *", but that results in an
- * unreasonable amount of extra casting both here and in callers, so we don't.
*/
float8
-float8in_internal(char *num, char **endptr_p,
+float8in_internal(const char *num, char **endptr_p,
const char *type_name, const char
*orig_string,
struct Node *escontext)
{
double val;
- char *endptr;
+ const char *endptr;
/* skip leading whitespace */
while (*num != '\0' && isspace((unsigned char) *num))
@@ -455,7 +452,7 @@ float8in_internal(char *num, char **endptr_p,
type_name, orig_string)));
errno = 0;
- val = strtod(num, &endptr);
+ val = strtod(num, unconstify(char **, &endptr));
/* did we not see anything that looks like a double? */
if (endptr == num || errno != 0)
@@ -544,7 +541,7 @@ float8in_internal(char *num, char **endptr_p,
/* report stopping point if wanted, else complain if not end of string
*/
if (endptr_p)
- *endptr_p = endptr;
+ *endptr_p = unconstify(char *, endptr);
else if (*endptr != '\0')
ereturn(escontext, 0,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
diff --git a/src/include/utils/float.h b/src/include/utils/float.h
index ffa743d6273..dbfbb3bf6ad 100644
--- a/src/include/utils/float.h
+++ b/src/include/utils/float.h
@@ -37,10 +37,10 @@ extern float8 float_overflow_error_ext(struct Node
*escontext);
extern float8 float_underflow_error_ext(struct Node *escontext);
extern float8 float_zero_divide_error_ext(struct Node *escontext);
extern int is_infinite(float8 val);
-extern float8 float8in_internal(char *num, char **endptr_p,
+extern float8 float8in_internal(const char *num, char **endptr_p,
const char
*type_name, const char *orig_string,
struct Node
*escontext);
-extern float4 float4in_internal(char *num, char **endptr_p,
+extern float4 float4in_internal(const char *num, char **endptr_p,
const char
*type_name, const char *orig_string,
struct Node
*escontext);
extern char *float8out_internal(float8 num);
--
2.56.0
From d8f2073cfd5be6b70d38dc4f333ce72d405177b5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 7 Oct 2026 08:21:37 +0200
Subject: [PATCH v3 2/2] Fix casting away of volatile qualifier
Fix some issues involving volatile that would be triggered by
-Wcast-qual.
For PMSignalState, move the volatile qualifiers to the PMSignalData
fields that need it rather than making the whole structure have it.
For the MemSet() calls, do the initialization honestly with a loop
instead.
Note that the MemSet() calls would be normal "discards qualifier"
warnings if memset() (or another function with a prototype, not a
macro) were used.
Discussion:
https://www.postgresql.org/message-id/c76b72dd-5736-453a-b698-6968d42e4768%40eisentraut.org
---
src/backend/postmaster/launch_backend.c | 2 +-
src/backend/storage/ipc/pmsignal.c | 8 ++++----
src/backend/storage/ipc/procsignal.c | 6 ++++--
src/backend/utils/activity/backend_progress.c | 3 ++-
src/include/storage/pmsignal.h | 2 +-
5 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/backend/postmaster/launch_backend.c
b/src/backend/postmaster/launch_backend.c
index 8f3cfea880c..093ef879120 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -105,7 +105,7 @@ typedef struct
PROC_HDR *ProcGlobal;
PGPROC *AuxiliaryProcs;
PGPROC *PreparedXactProcs;
- volatile PMSignalData *PMSignalState;
+ PMSignalData *PMSignalState;
ProcSignalHeader *ProcSignal;
pid_t PostmasterPid;
TimestampTz PgStartTime;
diff --git a/src/backend/storage/ipc/pmsignal.c
b/src/backend/storage/ipc/pmsignal.c
index bdad5fdd043..08d382d73ab 100644
--- a/src/backend/storage/ipc/pmsignal.c
+++ b/src/backend/storage/ipc/pmsignal.c
@@ -73,16 +73,16 @@
struct PMSignalData
{
/* per-reason flags for signaling the postmaster */
- sig_atomic_t PMSignalFlags[NUM_PMSIGNALS];
+ volatile sig_atomic_t PMSignalFlags[NUM_PMSIGNALS];
/* global flags for signals from postmaster to children */
- QuitSignalReason sigquit_reason; /* why SIGQUIT was sent */
+ volatile QuitSignalReason sigquit_reason; /* why SIGQUIT was sent
*/
/* per-child-process flags */
int num_child_flags; /* # of entries in
PMChildFlags[] */
- sig_atomic_t PMChildFlags[FLEXIBLE_ARRAY_MEMBER];
+ volatile sig_atomic_t PMChildFlags[FLEXIBLE_ARRAY_MEMBER];
};
/* PMSignalState pointer is valid in both postmaster and child processes */
-NON_EXEC_STATIC volatile PMSignalData *PMSignalState = NULL;
+NON_EXEC_STATIC PMSignalData *PMSignalState = NULL;
static void PMSignalShmemRequest(void *);
static void PMSignalShmemInit(void *);
diff --git a/src/backend/storage/ipc/procsignal.c
b/src/backend/storage/ipc/procsignal.c
index 21a77f98c1d..4dc9d96cd21 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -155,7 +155,8 @@ ProcSignalShmemInit(void *arg)
SpinLockInit(&slot->pss_mutex);
pg_atomic_init_u32(&slot->pss_pid, 0);
slot->pss_cancel_key_len = 0;
- MemSet(slot->pss_signalFlags, 0, sizeof(slot->pss_signalFlags));
+ for (int i = 0; i < NUM_PROCSIGNALS; i++)
+ slot->pss_signalFlags[i] = false;
pg_atomic_init_u64(&slot->pss_barrierGeneration, PG_UINT64_MAX);
pg_atomic_init_u32(&slot->pss_barrierCheckMask, 0);
ConditionVariableInit(&slot->pss_barrierCV);
@@ -186,7 +187,8 @@ ProcSignalInit(const uint8 *cancel_key, int cancel_key_len)
old_pss_pid = pg_atomic_read_u32(&slot->pss_pid);
/* Clear out any leftover signal reasons */
- MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS *
sizeof(sig_atomic_t));
+ for (int i = 0; i < NUM_PROCSIGNALS; i++)
+ slot->pss_signalFlags[i] = false;
/*
* Publish the PID before reading the global barrier generation to
ensure
diff --git a/src/backend/utils/activity/backend_progress.c
b/src/backend/utils/activity/backend_progress.c
index dee05b1abb1..32b2cdd81cd 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -35,7 +35,8 @@ pgstat_progress_start_command(ProgressCommandType cmdtype,
Oid relid)
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
beentry->st_progress_command = cmdtype;
beentry->st_progress_command_target = relid;
- MemSet(&beentry->st_progress_param, 0,
sizeof(beentry->st_progress_param));
+ for (int i = 0; i < PGSTAT_NUM_PROGRESS_PARAM; i++)
+ beentry->st_progress_param[i] = 0;
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h
index bcce4011790..1b1f7ddc480 100644
--- a/src/include/storage/pmsignal.h
+++ b/src/include/storage/pmsignal.h
@@ -61,7 +61,7 @@ typedef enum
typedef struct PMSignalData PMSignalData;
#ifdef EXEC_BACKEND
-extern PGDLLIMPORT volatile PMSignalData *PMSignalState;
+extern PGDLLIMPORT PMSignalData *PMSignalState;
#endif
/*
--
2.56.0
