Hi,
On Mon, Dec 22, 2025 at 12:53:03PM +0100, Peter Eisentraut wrote:
> On 18.12.25 14:55, Bertrand Drouvot wrote:
> > Some functions are casting away the const qualifiers from their signatures
> > in
> > local variables.
>
> @@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
> static int
> compare_distances(const void *a, const void *b)
> {
> - DistanceValue *da = (DistanceValue *) a;
> - DistanceValue *db = (DistanceValue *) b;
> + const DistanceValue *da = (const DistanceValue *) a;
> + const DistanceValue *db = (const DistanceValue *) b;
>
> I wonder if the better fix here wouldn't be to get rid of the cast. It's not
> necessary, and without it the compiler would automatically warn about
> qualifier mismatches.
Yeah, that looks better as it provides an extra safety check should the function
signature change.
> These comparison functions seem to be a common
> pattern.
Right, in the attached I applied your proposal on all those places.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From aa228ca767d9f1cfd6785ea5387ea37db2a560c0 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 18 Dec 2025 09:51:01 +0000
Subject: [PATCH v2 1/3] Don't cast away const where possible
Add const to read only local variables, preserving the const qualifiers from the
function signatures.
This does not change all such instances, but only those hand-picked by the author.
The ones that are not changed:
- are just thin wrappers
- would require public API changes
- rely on external functions (such as LZ4F_compressUpdate())
- would require complex subsystem changes
Discussion: https://postgr.es/m/aUQHy/MmWq7c97wK%40ip-10-97-1-34.eu-west-3.compute.internal
---
src/backend/access/brin/brin_minmax_multi.c | 4 ++--
src/backend/access/heap/pruneheap.c | 4 ++--
src/backend/access/spgist/spgkdtreeproc.c | 8 ++++----
src/backend/statistics/mcv.c | 4 ++--
src/backend/tsearch/spell.c | 4 ++--
src/test/modules/injection_points/injection_points.c | 8 ++++----
6 files changed, 16 insertions(+), 16 deletions(-)
10.2% src/backend/access/brin/
12.5% src/backend/access/heap/
18.7% src/backend/access/spgist/
8.3% src/backend/statistics/
11.6% src/backend/tsearch/
38.4% src/test/modules/injection_points/
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 0298a9da8ba..0e87ce759f1 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
static int
compare_distances(const void *a, const void *b)
{
- DistanceValue *da = (DistanceValue *) a;
- DistanceValue *db = (DistanceValue *) b;
+ const DistanceValue *da = a;
+ const DistanceValue *db = b;
if (da->value < db->value)
return 1;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 07aa08cfe14..92aab1d6723 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -2021,8 +2021,8 @@ heap_log_freeze_eq(xlhp_freeze_plan *plan, HeapTupleFreeze *frz)
static int
heap_log_freeze_cmp(const void *arg1, const void *arg2)
{
- HeapTupleFreeze *frz1 = (HeapTupleFreeze *) arg1;
- HeapTupleFreeze *frz2 = (HeapTupleFreeze *) arg2;
+ const HeapTupleFreeze *frz1 = arg1;
+ const HeapTupleFreeze *frz2 = arg2;
if (frz1->xmax < frz2->xmax)
return -1;
diff --git a/src/backend/access/spgist/spgkdtreeproc.c b/src/backend/access/spgist/spgkdtreeproc.c
index f0167d6ffa6..c02de835847 100644
--- a/src/backend/access/spgist/spgkdtreeproc.c
+++ b/src/backend/access/spgist/spgkdtreeproc.c
@@ -84,8 +84,8 @@ typedef struct SortedPoint
static int
x_cmp(const void *a, const void *b)
{
- SortedPoint *pa = (SortedPoint *) a;
- SortedPoint *pb = (SortedPoint *) b;
+ const SortedPoint *pa = a;
+ const SortedPoint *pb = b;
if (pa->p->x == pb->p->x)
return 0;
@@ -95,8 +95,8 @@ x_cmp(const void *a, const void *b)
static int
y_cmp(const void *a, const void *b)
{
- SortedPoint *pa = (SortedPoint *) a;
- SortedPoint *pb = (SortedPoint *) b;
+ const SortedPoint *pa = a;
+ const SortedPoint *pb = b;
if (pa->p->y == pb->p->y)
return 0;
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index ec650ba029f..dc577b9a1e4 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -402,8 +402,8 @@ count_distinct_groups(int numrows, SortItem *items, MultiSortSupport mss)
static int
compare_sort_item_count(const void *a, const void *b, void *arg)
{
- SortItem *ia = (SortItem *) a;
- SortItem *ib = (SortItem *) b;
+ const SortItem *ia = a;
+ const SortItem *ib = b;
if (ia->count == ib->count)
return 0;
diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index e5badb6b43f..3b0abfd9c35 100644
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -210,8 +210,8 @@ cmpspellaffix(const void *s1, const void *s2)
static int
cmpcmdflag(const void *f1, const void *f2)
{
- CompoundAffixFlag *fv1 = (CompoundAffixFlag *) f1,
- *fv2 = (CompoundAffixFlag *) f2;
+ const CompoundAffixFlag *fv1 = f1;
+ const CompoundAffixFlag *fv2 = f2;
Assert(fv1->flagMode == fv2->flagMode);
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 25340e8d81b..12ea81a030c 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -189,7 +189,7 @@ injection_init_shmem(void)
* otherwise.
*/
static bool
-injection_point_allowed(InjectionPointCondition *condition)
+injection_point_allowed(const InjectionPointCondition *condition)
{
bool result = true;
@@ -232,7 +232,7 @@ injection_points_cleanup(int code, Datum arg)
void
injection_error(const char *name, const void *private_data, void *arg)
{
- InjectionPointCondition *condition = (InjectionPointCondition *) private_data;
+ const InjectionPointCondition *condition = private_data;
char *argstr = (char *) arg;
if (!injection_point_allowed(condition))
@@ -248,7 +248,7 @@ injection_error(const char *name, const void *private_data, void *arg)
void
injection_notice(const char *name, const void *private_data, void *arg)
{
- InjectionPointCondition *condition = (InjectionPointCondition *) private_data;
+ const InjectionPointCondition *condition = private_data;
char *argstr = (char *) arg;
if (!injection_point_allowed(condition))
@@ -268,7 +268,7 @@ injection_wait(const char *name, const void *private_data, void *arg)
uint32 old_wait_counts = 0;
int index = -1;
uint32 injection_wait_event = 0;
- InjectionPointCondition *condition = (InjectionPointCondition *) private_data;
+ const InjectionPointCondition *condition = private_data;
if (inj_state == NULL)
injection_init_shmem();
--
2.34.1
>From 3c8b291ddd9cce04850b36002befcaf5d9daee57 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 18 Dec 2025 12:47:07 +0000
Subject: [PATCH v2 2/3] Add const to read only TableInfo pointers in pg_dump
Functions that dump table data receive their parameters through const void *
but were casting away const. Add const qualifiers to functions that only read
the table information.
Also change getRootTableInfo to return const TableInfo *, since it only traverses
the parent chain without modifying any TableInfo structures. This allows the dump
functions to maintain const correctness when calling it.
Discussion: https://postgr.es/m/aUQHy/MmWq7c97wK%40ip-10-97-1-34.eu-west-3.compute.internal
---
src/bin/pg_dump/pg_dump.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
100.0% src/bin/pg_dump/
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 27f6be3f0f8..3f5e917ad99 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -412,7 +412,7 @@ static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
static char *get_synchronized_snapshot(Archive *fout);
static void set_restrict_relation_kind(Archive *AH, const char *value);
static void setupDumpWorker(Archive *AH);
-static TableInfo *getRootTableInfo(const TableInfo *tbinfo);
+static const TableInfo *getRootTableInfo(const TableInfo *tbinfo);
static bool forcePartitionRootLoad(const TableInfo *tbinfo);
static void read_dump_filters(const char *filename, DumpOptions *dopt);
@@ -2379,8 +2379,8 @@ selectDumpableObject(DumpableObject *dobj, Archive *fout)
static int
dumpTableData_copy(Archive *fout, const void *dcontext)
{
- TableDataInfo *tdinfo = (TableDataInfo *) dcontext;
- TableInfo *tbinfo = tdinfo->tdtable;
+ const TableDataInfo *tdinfo = dcontext;
+ const TableInfo *tbinfo = tdinfo->tdtable;
const char *classname = tbinfo->dobj.name;
PQExpBuffer q = createPQExpBuffer();
@@ -2547,8 +2547,8 @@ dumpTableData_copy(Archive *fout, const void *dcontext)
static int
dumpTableData_insert(Archive *fout, const void *dcontext)
{
- TableDataInfo *tdinfo = (TableDataInfo *) dcontext;
- TableInfo *tbinfo = tdinfo->tdtable;
+ const TableDataInfo *tdinfo = dcontext;
+ const TableInfo *tbinfo = tdinfo->tdtable;
DumpOptions *dopt = fout->dopt;
PQExpBuffer q = createPQExpBuffer();
PQExpBuffer insertStmt = NULL;
@@ -2618,7 +2618,7 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
*/
if (insertStmt == NULL)
{
- TableInfo *targettab;
+ const TableInfo *targettab;
insertStmt = createPQExpBuffer();
@@ -2813,10 +2813,10 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
* getRootTableInfo:
* get the root TableInfo for the given partition table.
*/
-static TableInfo *
+static const TableInfo *
getRootTableInfo(const TableInfo *tbinfo)
{
- TableInfo *parentTbinfo;
+ const TableInfo *parentTbinfo;
Assert(tbinfo->ispartition);
Assert(tbinfo->numParents == 1);
@@ -2870,7 +2870,7 @@ static void
dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
{
DumpOptions *dopt = fout->dopt;
- TableInfo *tbinfo = tdinfo->tdtable;
+ const TableInfo *tbinfo = tdinfo->tdtable;
PQExpBuffer copyBuf = createPQExpBuffer();
PQExpBuffer clistBuf = createPQExpBuffer();
DataDumperPtr dumpFn;
@@ -2891,7 +2891,7 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
(dopt->load_via_partition_root ||
forcePartitionRootLoad(tbinfo)))
{
- TableInfo *parentTbinfo;
+ const TableInfo *parentTbinfo;
char *sanitized;
parentTbinfo = getRootTableInfo(tbinfo);
--
2.34.1
>From 4fa3127b65fc6237e6f63e5d5ad0e6486ae40a38 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 18 Dec 2025 12:17:21 +0000
Subject: [PATCH v2 3/3] Separate read and write pointers in pg_saslprep
Use separate pointers for reading const input ('p') and writing
to mutable output ('outp'), avoiding the need to cast away const
on the input parameter.
Discussion: https://postgr.es/m/aUQHy/MmWq7c97wK%40ip-10-97-1-34.eu-west-3.compute.internal
---
src/common/saslprep.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
100.0% src/common/
diff --git a/src/common/saslprep.c b/src/common/saslprep.c
index 101e8d65a4d..b215ab0bf8e 100644
--- a/src/common/saslprep.c
+++ b/src/common/saslprep.c
@@ -1054,7 +1054,8 @@ pg_saslprep(const char *input, char **output)
int count;
int i;
bool contains_RandALCat;
- unsigned char *p;
+ const unsigned char *p;
+ unsigned char *outp;
char32_t *wp;
/* Ensure we return *output as NULL on failure */
@@ -1087,7 +1088,7 @@ pg_saslprep(const char *input, char **output)
if (!input_chars)
goto oom;
- p = (unsigned char *) input;
+ p = (const unsigned char *) input;
for (i = 0; i < input_size; i++)
{
input_chars[i] = utf8_to_unicode(p);
@@ -1217,14 +1218,14 @@ pg_saslprep(const char *input, char **output)
* There are no error exits below here, so the error exit paths don't need
* to worry about possibly freeing "result".
*/
- p = (unsigned char *) result;
+ outp = (unsigned char *) result;
for (wp = output_chars; *wp; wp++)
{
- unicode_to_utf8(*wp, p);
- p += pg_utf_mblen(p);
+ unicode_to_utf8(*wp, outp);
+ outp += pg_utf_mblen(outp);
}
- Assert((char *) p == result + result_size);
- *p = '\0';
+ Assert((char *) outp == result + result_size);
+ *outp = '\0';
FREE(input_chars);
FREE(output_chars);
--
2.34.1
