Обсуждение: Fix accidentally cast away qualifiers
This patch fixes cases where a qualifier (const, in all cases here) was dropped by a cast, but the cast was otherwise necessary or desirable, so the straightforward fix is to add the qualifier into the cast. This was checked with gcc -Wcast-qual, but it doesn't fix all such warnings, only the trivially fixable ones.
Вложения
> On Jan 20, 2026, at 15:54, Peter Eisentraut <peter@eisentraut.org> wrote: > > This patch fixes cases where a qualifier (const, in all cases here) was dropped by a cast, but the cast was otherwise necessaryor desirable, so the straightforward fix is to add the qualifier into the cast. > > This was checked with gcc -Wcast-qual, but it doesn't fix all such warnings, only the trivially fixable ones.<0001-Fix-accidentally-cast-away-qualifiers.patch> Looks good to me. All changes are const-correctness fixes without semantic changes. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Hi,
On Tue, Jan 20, 2026 at 08:54:18AM +0100, Peter Eisentraut wrote:
> This patch fixes cases where a qualifier (const, in all cases here) was
> dropped by a cast, but the cast was otherwise necessary or desirable, so the
> straightforward fix is to add the qualifier into the cast.
>
> This was checked with gcc -Wcast-qual, but it doesn't fix all such warnings,
> only the trivially fixable ones.
diff --git a/src/include/varatt.h b/src/include/varatt.h
index eccd3ca04d6..03e9d1869aa 100644
--- a/src/include/varatt.h
+++ b/src/include/varatt.h
It looks like those changes produce:
../../../src/include/varatt.h: In function ‘VARDATA’:
../../../src/include/varatt.h:261:33: warning: return discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
261 | #define VARDATA_4B(PTR) (((const varattrib_4b *) (PTR))->va_4byte.va_data)
| ^
../../../src/include/varatt.h:307:16: note: in expansion of macro ‘VARDATA_4B’
307 | return VARDATA_4B(PTR);
| ^~~~~~~~~~
../../../src/include/varatt.h: In function ‘VARDATA_SHORT’:
../../../src/include/varatt.h:263:33: warning: return discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
263 | #define VARDATA_1B(PTR) (((const varattrib_1b *) (PTR))->va_data)
| ^
../../../src/include/varatt.h:321:16: note: in expansion of macro ‘VARDATA_1B’
321 | return VARDATA_1B(PTR);
| ^~~~~~~~~~
../../../src/include/varatt.h: In function ‘VARDATA_EXTERNAL’:
../../../src/include/varatt.h:264:33: warning: return discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
264 | #define VARDATA_1B_E(PTR) (((const varattrib_1b_e *) (PTR))->va_data)
| ^
../../../src/include/varatt.h:342:16: note: in expansion of macro ‘VARDATA_1B_E’
342 | return VARDATA_1B_E(PTR);
| ^~~~~~~~~~~~
../../../src/include/varatt.h: In function ‘VARDATA_ANY’:
../../../src/include/varatt.h:488:52: warning: return discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
488 | return VARATT_IS_1B(PTR) ? VARDATA_1B(PTR) : VARDATA_4B(PTR);
as they are still used by functions that return non const:
"
static inline char *
VARDATA(const void *PTR)
{
return VARDATA_4B(PTR);
}
"
> - RangeType *r1 = *(RangeType **) key1;
> - RangeType *r2 = *(RangeType **) key2;
> + RangeType *r1 = *(RangeType *const *) key1;
> + RangeType *r2 = *(RangeType *const *) key2;
That's correct. My improved cocci script [1] would incorrectly produce:
- RangeType *r1 = *(RangeType **) key1;
- RangeType *r2 = *(RangeType **) key2;
+ RangeType *r1 = *(const RangeType **)key1;
+ RangeType *r2 = *(const RangeType **)key2;
Same for:
> - file_entry_t *fa = *((file_entry_t **) a);
> - file_entry_t *fb = *((file_entry_t **) b);
> + file_entry_t *fa = *((file_entry_t *const *) a);
> + file_entry_t *fb = *((file_entry_t *const *) b);
>
>
Same for:
> - Step *stepa = *((Step **) a);
> - Step *stepb = *((Step **) b);
> + Step *stepa = *((Step *const *) a);
> + Step *stepb = *((Step *const *) b);
>
> - Step *step = *((Step **) b);
> + Step *step = *((Step *const *) b);
>
> diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
> index 03371721460..4a9bb65b9bb 100644
> --- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
> +++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
> @@ -1576,7 +1576,7 @@ test_singlerowmode(PGconn *conn)
> "SELECT generate_series(42, $1)",
> 1,
> NULL,
> - (const char **) param,
> + (const char *const *) param,
> NULL,
> NULL,
> 0) != 1)
Correct, also according to:
"
int
PQsendQueryParams(PGconn *conn,
const char *command,
int nParams,
const Oid *paramTypes,
const char *const *paramValues,
const int *paramLengths,
const int *paramFormats,
int resultFormat)
{
"
Also [1], detected a few more trivially fixable ones (see attached).
[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/search_const_away.cocci
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Вложения
On 20.01.26 13:10, Bertrand Drouvot wrote: > On Tue, Jan 20, 2026 at 08:54:18AM +0100, Peter Eisentraut wrote: >> This patch fixes cases where a qualifier (const, in all cases here) was >> dropped by a cast, but the cast was otherwise necessary or desirable, so the >> straightforward fix is to add the qualifier into the cast. >> >> This was checked with gcc -Wcast-qual, but it doesn't fix all such warnings, >> only the trivially fixable ones. > > diff --git a/src/include/varatt.h b/src/include/varatt.h > index eccd3ca04d6..03e9d1869aa 100644 > --- a/src/include/varatt.h > +++ b/src/include/varatt.h > > It looks like those changes produce: > > ../../../src/include/varatt.h: In function ‘VARDATA’: > ../../../src/include/varatt.h:261:33: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > 261 | #define VARDATA_4B(PTR) (((const varattrib_4b *) (PTR))->va_4byte.va_data) Strange, I don't see that. What compiler is this, and do you use any special options? > Also [1], detected a few more trivially fixable ones (see attached). Yes, these should be included. The one in spgquadtreeproc.c was #ifdef'ed out, so my testing didn't see it. I suppose the other one is only compiled when you run the unicode tests.
Hi, On Wed, Jan 21, 2026 at 12:31:27PM +0100, Peter Eisentraut wrote: > On 20.01.26 13:10, Bertrand Drouvot wrote: > > On Tue, Jan 20, 2026 at 08:54:18AM +0100, Peter Eisentraut wrote: > > > This patch fixes cases where a qualifier (const, in all cases here) was > > > dropped by a cast, but the cast was otherwise necessary or desirable, so the > > > straightforward fix is to add the qualifier into the cast. > > > > > > This was checked with gcc -Wcast-qual, but it doesn't fix all such warnings, > > > only the trivially fixable ones. > > > > diff --git a/src/include/varatt.h b/src/include/varatt.h > > index eccd3ca04d6..03e9d1869aa 100644 > > --- a/src/include/varatt.h > > +++ b/src/include/varatt.h > > > > It looks like those changes produce: > > > > ../../../src/include/varatt.h: In function ‘VARDATA’: > > ../../../src/include/varatt.h:261:33: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > > 261 | #define VARDATA_4B(PTR) (((const varattrib_4b *) (PTR))->va_4byte.va_data) > > Strange, I don't see that. What compiler is this, and do you use any > special options? It's gcc 14.1.0 with "CFLAGS=-O0 -ggdb3 -fno-omit-frame-pointer -gdwarf-2 -g3 -Wdiscarded-qualifiers -Wunused-value -Werror=maybe-uninitialized -Werror=format -Wreturn-type" But I just tested with a simple test case, some compilers and some options and I've always seen the warning (see [1]), so I'm not sure why you don't. > > > Also [1], detected a few more trivially fixable ones (see attached). > > Yes, these should be included. > > The one in spgquadtreeproc.c was #ifdef'ed out, so my testing didn't see it. > I suppose the other one is only compiled when you run the unicode tests. Yeah, that could lead to the same "concern" that Tom had in [2] (and using a tool like Coccinelle could be an answer). [1]: https://godbolt.org/z/T9hffKaPW [2]: https://postgr.es/m/aVJCoNqO6ybov7eW%40ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Hi, On Wed, Jan 21, 2026 at 02:24:58PM +0000, Bertrand Drouvot wrote: > Hi, > > On Wed, Jan 21, 2026 at 12:31:27PM +0100, Peter Eisentraut wrote: > > Strange, I don't see that. What compiler is this, and do you use any > > special options? > > It's gcc 14.1.0 with "CFLAGS=-O0 -ggdb3 -fno-omit-frame-pointer -gdwarf-2 -g3 > -Wdiscarded-qualifiers -Wunused-value -Werror=maybe-uninitialized > -Werror=format -Wreturn-type" > > But I just tested with a simple test case, some compilers and some options and I've > always seen the warning (see [1]) FWIW, and so does the CI: https://github.com/bdrouvot/postgres/runs/61028037498 Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
> On Jan 21, 2026, at 22:56, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > Hi, > > On Wed, Jan 21, 2026 at 02:24:58PM +0000, Bertrand Drouvot wrote: >> Hi, >> >> On Wed, Jan 21, 2026 at 12:31:27PM +0100, Peter Eisentraut wrote: >>> Strange, I don't see that. What compiler is this, and do you use any >>> special options? >> >> It's gcc 14.1.0 with "CFLAGS=-O0 -ggdb3 -fno-omit-frame-pointer -gdwarf-2 -g3 >> -Wdiscarded-qualifiers -Wunused-value -Werror=maybe-uninitialized >> -Werror=format -Wreturn-type" >> >> But I just tested with a simple test case, some compilers and some options and I've >> always seen the warning (see [1]) > > FWIW, and so does the CI: https://github.com/bdrouvot/postgres/runs/61028037498 > > Regards, > > -- > Bertrand Drouvot > PostgreSQL Contributors Team > RDS Open Source Databases > Amazon Web Services: https://aws.amazon.com > Yeah, I confirm I can see the same warnings: ``` In file included from xlogdesc.c:21: In file included from ../../../src/include/utils/guc.h:17: In file included from ../../../src/include/utils/array.h:65: In file included from ../../../src/include/utils/expandeddatum.h:47: ../../../src/include/varatt.h:307:9: warning: returning 'const char[]' from a function with result type 'char *' discardsqualifiers [-Wincompatible-pointer-types-discards-qualifiers] 307 | return VARDATA_4B(PTR); | ^~~~~~~~~~~~~~~ ../../../src/include/varatt.h:261:26: note: expanded from macro 'VARDATA_4B' 261 | #define VARDATA_4B(PTR) (((const varattrib_4b *) (PTR))->va_4byte.va_data) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../../src/include/varatt.h:321:9: warning: returning 'const char[]' from a function with result type 'char *' discardsqualifiers [-Wincompatible-pointer-types-discards-qualifiers] 321 | return VARDATA_1B(PTR); | ^~~~~~~~~~~~~~~ ../../../src/include/varatt.h:263:26: note: expanded from macro 'VARDATA_1B' 263 | #define VARDATA_1B(PTR) (((const varattrib_1b *) (PTR))->va_data) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../../src/include/varatt.h:342:9: warning: returning 'const char[]' from a function with result type 'char *' discardsqualifiers [-Wincompatible-pointer-types-discards-qualifiers] 342 | return VARDATA_1B_E(PTR); | ^~~~~~~~~~~~~~~~~ ../../../src/include/varatt.h:264:27: note: expanded from macro 'VARDATA_1B_E' 264 | #define VARDATA_1B_E(PTR) (((const varattrib_1b_e *) (PTR))->va_data) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../../src/include/varatt.h:488:9: warning: returning 'const char *' from a function with result type 'char *' discardsqualifiers [-Wincompatible-pointer-types-discards-qualifiers] 488 | return VARATT_IS_1B(PTR) ? VARDATA_1B(PTR) : VARDATA_4B(PTR); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../../src/include/varatt.h:235:2: note: expanded from macro 'VARATT_IS_1B' 235 | ((((const varattrib_1b *) (PTR))->va_header & 0x01) == 0x01) | ^ 4 warnings generated. ``` I didn’t use any special compiler option, just ran “make”. My configure is: ``` ./configure --enable-debug --enable-cassert --enable-tap-tests --enable-depend ``` Sorry, I didn’t notice the warnings in my first review. I guess I left my desktop while building at the time. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On 21.01.26 15:56, Bertrand Drouvot wrote: > Hi, > > On Wed, Jan 21, 2026 at 02:24:58PM +0000, Bertrand Drouvot wrote: >> Hi, >> >> On Wed, Jan 21, 2026 at 12:31:27PM +0100, Peter Eisentraut wrote: >>> Strange, I don't see that. What compiler is this, and do you use any >>> special options? >> >> It's gcc 14.1.0 with "CFLAGS=-O0 -ggdb3 -fno-omit-frame-pointer -gdwarf-2 -g3 >> -Wdiscarded-qualifiers -Wunused-value -Werror=maybe-uninitialized >> -Werror=format -Wreturn-type" >> >> But I just tested with a simple test case, some compilers and some options and I've >> always seen the warning (see [1]) > > FWIW, and so does the CI: https://github.com/bdrouvot/postgres/runs/61028037498 I must have done something wrong on my side. I have committed it with your corrections.