Re: -Wcast-qual home stretch

> On Nov 3, 2026, at 23:34, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> We have been progressively cleaning up -Wcast-qual warnings this year. This patch set is the final part of this, containing a few tricky, complicated, and/or ugly cases that I have been putting off. And then we can turn -Wcast-qual on by default. Each patch is individually described in its commit message. Alternative approaches are possible; feedback welcome.<0001-Fix-accidentally-casting-away-const.patch><0002-Fix-Wcast-qual-warning-in-lnext.patch><0003-Fix-Wcast-qual-warning-in-foreach_delete_current.patch><0004-Fix-Wcast-qual-warning-in-XLogInsertRecord.patch><0005-Fix-Wcast-qual-warnings-in-regex-module.patch><0006-Fix-Wcast-qual-warning-in-guc_funcs.c.patch><0007-Add-unconstify_pointee.patch><0008-Fix-Wcast-qual-warnings-in-read-write-accessor-macro.patch><0009-Adjust-unconstify-unvolatize-to-avoid-Wcast-qual-war.patch><0010-Add-warning-option-Wcast-qual.patch>

The patch set overall looks good to me. I have a few comments as below:

1 - 0001
```
--- a/src/include/access/detoast.h
+++ b/src/include/access/detoast.h
@@ -23,7 +23,7 @@
*/
#define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \
do { \
- varattrib_1b_e *attre = (varattrib_1b_e *) (attr); \
+ const varattrib_1b_e *attre = (const varattrib_1b_e *) (attr); \
```

Looks like the header comment needs to be updated accordingly:
```
* intermediate "varattrib_1b_e *" variable seems to fix it.
```

2 - 0002
```
static inline ListCell *
lnext(const List *l, const ListCell *c)
{
- Assert(c >= &l->elements[0] && c < &l->elements[l->length]);
- c++;
- if (c < &l->elements[l->length])
- return (ListCell *) c;
+ int n = list_cell_number(l, c) + 1;
+
+ if (n < l->length)
+ return &l->elements[n];
else
return NULL;
}
```

Here, I think the problem is that c is const while the return value is not const. Can't we just use unconstify(), like:
```
return unconstify(ListCell *, c);
```

Feels like VARDATA_EXTERNAL is similar use case that is used in 0001. It calls VARDATA_EXTERNAL(attre), where attre is const, and VARDATA_EXTERNAL() uses unconstify() to make it non-const.

3 - 0006

Since BuildTupleFromCStrings doesn’t update values, making values as a const feels more reasonable, like:
```
extern HeapTuple BuildTupleFromCStrings(AttInMetadata *attinmeta, const char *const*values);
```

I tried that, but that required more callers’ changes. So, I’d reserve this comment, maybe we can do that with a separate refactoring patch.

4 - After applying the patch set, my local build still gets a couple of warnings:
```
input.c:306:27: warning: cast from 'const char *' to 'char *' drops const qualifier [-Wcast-qual]
306 | for (cur_ptr = (char *) cur_hist->line; *cur_ptr; cur_ptr++)
| ^
input.c:326:27: warning: cast from 'const char *' to 'char *' drops const qualifier [-Wcast-qual]
326 | for (cur_ptr = (char *) cur_hist->line; *cur_ptr; cur_ptr++)
| ^
2 warnings generated.
```

We can fix them by using unconstify:
```
diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index 6d37359fc96..177434ed975 100644
--- a/src/bin/psql/input.c
+++ b/src/bin/psql/input.c
@@ -303,7 +303,7 @@ encode_history(void)
char *cur_ptr;

/* some platforms declare HIST_ENTRY.line as const char * */
- for (cur_ptr = (char *) cur_hist->line; *cur_ptr; cur_ptr++)
+ for (cur_ptr = unconstify(char *, cur_hist->line); *cur_ptr; cur_ptr++)
{
if (*cur_ptr == '\n')
*cur_ptr = NL_IN_HISTORY;
@@ -323,7 +323,7 @@ decode_history(void)
char *cur_ptr;

/* some platforms declare HIST_ENTRY.line as const char * */
- for (cur_ptr = (char *) cur_hist->line; *cur_ptr; cur_ptr++)
+ for (cur_ptr = unconstify(char *, cur_hist->line); *cur_ptr; cur_ptr++)
{
if (*cur_ptr == NL_IN_HISTORY)
*cur_ptr = '\n';
```

But that doesn’t feel good, because we are writing to the pointers, forcing to unconstify those points might be unsafe.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/