I have committed the first few of these. (The compiler warning flag
itself is not activated yet.) This should allow you to proceed with
your patches that add various const qualifiers. I'll come back to the
rest later.
On 18.06.24 17:02, Andres Freund wrote:
>> diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
>> index 07bf356b70c..5a124385b7c 100644
>> --- a/src/bin/pg_archivecleanup/pg_archivecleanup.c
>> +++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c
>> @@ -19,17 +19,18 @@
>> #include "common/logging.h"
>> #include "getopt_long.h"
>>
>> -const char *progname;
>> +static const char *progname;
>
> Hm, this one I'm not so sure about. The backend version is explicitly globally
> visible, and I don't see why we shouldn't do the same for other binaries.
We have in various programs a mix of progname with static linkage and
with external linkage. AFAICT, this is merely determined by whether
there are multiple source files that need it, not by some higher-level
scheme.
>> From d89312042eb76c879d699380a5e2ed0bc7956605 Mon Sep 17 00:00:00 2001
>> From: Peter Eisentraut <peter@eisentraut.org>
>> Date: Sun, 16 Jun 2024 23:52:06 +0200
>> Subject: [PATCH v2 05/10] Fix warnings from -Wmissing-variable-declarations
>> under EXEC_BACKEND
>>
>> The NON_EXEC_STATIC variables need a suitable declaration in a header
>> file under EXEC_BACKEND.
>>
>> Also fix the inconsistent application of the volatile qualifier for
>> PMSignalState, which was revealed by this change.
>
> I'm very very unenthused about adding volatile to more places. It's rarely
> correct and often slow. But I guess this doesn't really make it any worse.
Yeah, it's not always clear with volatile, but in this one case it's
probably better to keep it consistent rather than having to cast it away
or something.
>> +#ifdef TRACE_SYNCSCAN
>> +#include "access/syncscan.h"
>> +#endif
>
> I'd just include it unconditionally.
My thinking here was that if we apply an include file cleaner (like
iwyu) sometime, it would flag this include as unused. This way it's
clearer what it's for.
