Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix some cases of indirectly casting away const.

  2. Fix another case of indirectly casting away const.

  3. ecpg: refactor to eliminate cast-away-const in find_variable().

  1. More const-marking cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-04T22:09:30Z

    I noticed that our two buildfarm animals that are running Fedora
rawhide (caiman, midge) are emitting warnings like

compression.c: In function "parse_compress_options":
compression.c:458:13: warning: assignment discards "const" qualifier from pointer target type [-Wdiscarded-qualifiers]
  458 |         sep = strchr(option, ':');
      |             ^

Apparently, latest gcc is able to notice that constructions like

	const char *str = ...;
	char       *ptr = strchr(str, ':');

are effectively casting away const.  This is a good thing and long
overdue, but we have some work to do to clean up the places where
we are doing that.

Attached is a patch that cleans up all the cases I saw on a local
rawhide installation.  Most of it is unremarkable, but the changes
in ecpg are less so.  In particular, variable.c is a mess, because
somebody const-ified some of the char* arguments to find_struct()
and find_struct_member() without regard for the fact that all their
char* arguments point at the same string.  (Not that you could
discover that from their nonexistent documentation.)  I considered
just reverting that change, but was able to fix things up reasonably
well at the price of a few extra strdup's.  It turned out that
find_struct_member()'s modification of its input string is actually
entirely useless, because it undoes that before consulting the
string again.  find_struct() and find_variable() need to make
copies, though.

			regards, tom lane

  2. Re: More const-marking cleanup

    Thomas Munro <thomas.munro@gmail.com> — 2025-12-04T22:52:35Z

    On Fri, Dec 5, 2025 at 11:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Apparently, latest gcc is able to notice that constructions like
>
>         const char *str = ...;
>         char       *ptr = strchr(str, ':');
>
> are effectively casting away const.  This is a good thing and long
> overdue, but we have some work to do to clean up the places where
> we are doing that.

Yeah, one of the qualifier-preserving generic functions that C23
invented: bsearch, memchr, strchr, strpbrk, strrchr, strstr, wcschr,
wcspbrk, wcsrchr, wmemchr, and wcsstr.  The synopses use QVoid or
QChar to mean "same qualifier", a bit like C++ function templates.  We
could probably benefit from some of that in our own code node, list,
tree etc code, as it only requires C11 _Generic to implement.

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf

  3. Re: More const-marking cleanup

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-05T14:31:00Z

    Hi,

On Thu, Dec 04, 2025 at 05:09:30PM -0500, Tom Lane wrote:
> Attached is a patch that cleans up all the cases I saw on a local
> rawhide installation. 

What about adding the 4 in the attached?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  4. Re: More const-marking cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-05T14:47:56Z

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> What about adding the 4 in the attached?

Cool, how'd you find these?

			regards, tom lane

  5. Re: More const-marking cleanup

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-05T14:51:45Z

    Hi,

On Fri, Dec 05, 2025 at 09:47:56AM -0500, Tom Lane wrote:
> Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> > What about adding the 4 in the attached?
> 
> Cool, how'd you find these?

With the help of a coccinelle script. Will polish and share.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  6. Re: More const-marking cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-05T16:20:50Z

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> On Fri, Dec 05, 2025 at 09:47:56AM -0500, Tom Lane wrote:
>> Cool, how'd you find these?

> With the help of a coccinelle script. Will polish and share.

Ah.  Your script didn't notice the need for const'ification of
additional variables in map_locale() :-(.  I fixed that and
pushed everything except the ecpg/preproc/variable.c changes,
which I'm not too comfortable about yet.  (The code coverage
report shows that large chunks of those functions are untested,
so I wonder if they worked before let alone after.)

			regards, tom lane

  7. Re: More const-marking cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-05T20:52:40Z

    I wrote:
> Ah.  Your script didn't notice the need for const'ification of
> additional variables in map_locale() :-(.  I fixed that and
> pushed everything except the ecpg/preproc/variable.c changes,
> which I'm not too comfortable about yet.  (The code coverage
> report shows that large chunks of those functions are untested,
> so I wonder if they worked before let alone after.)

I was right to be suspicious about variable.c: I'd misunderstood
what was happening in find_struct_member(), with the consequence
that I broke some cases that were not being tested.  Here's v2,
with that repaired, more commentary, and more test cases.

Adding these comments feels a bit like putting lipstick on a pig.
find_variable and its subroutines are an inelegant, duplicative
mess that fails to handle cases it easily could handle if it were
rewritten.  But I've put enough brain cells into this already,
and also it appears that there are restrictions elsewhere in ecpg
that'd have to be lifted before it'd make a difference.

			regards, tom lane

  8. Re: More const-marking cleanup

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-08T07:43:34Z

    Hi,

On Fri, Dec 05, 2025 at 03:52:40PM -0500, Tom Lane wrote:
> I wrote:
> > Ah.  Your script didn't notice the need for const'ification of
> > additional variables in map_locale() :-(.

Ah, right, thanks for letting me know. It would need more work to catch those.

That said I improved the script [1] so that:

- It found one more (see the attached)
- It is able to detect cases even for functions created in the code tree. It found
that skip_drive() (path.c) and bsearch_arg() (bsearch_arg.c) are also functions
that cast away const in their return values
- It also checked callers for the 2 functions above and did not find const to add

> Adding these comments feels a bit like putting lipstick on a pig.
> find_variable and its subroutines are an inelegant, duplicative
> mess that fails to handle cases it easily could handle if it were
> rewritten.  But I've put enough brain cells into this already,
> and also it appears that there are restrictions elsewhere in ecpg
> that'd have to be lifted before it'd make a difference.

I also look at those (even if already pushed in 4eda42e8bdf) and they LGTM.

[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/indirectly_casting_away_const.cocci

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  9. Re: More const-marking cleanup

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-17T10:03:28Z

    Hi,

On Mon, Dec 08, 2025 at 07:43:34AM +0000, Bertrand Drouvot wrote:
> That said I improved the script [1] so that:
> 
> - It found one more (see the attached)

I had another look and it seems to me that the one (src/port/getopt.c) reported
and fixed in the attached of the previous email is the only remaining one to fix.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  10. Re: More const-marking cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-24T02:47:55Z

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> I had another look and it seems to me that the one (src/port/getopt.c) reported
> and fixed in the attached of the previous email is the only remaining one to fix.

This fell off my radar for a bit, but pushed now.  Thanks for
catching it.

This does raise a question in my mind though: we did not find this
case the first time around because machines that have new-enough
compilers to detect such issues will probably not need our version of
getopt().  So, what other not-always-compiled bits of code could use
improvement, and how could we find them without undue effort?  (This
extends to more issues than just casting-away-const of course.)

			regards, tom lane

  11. Re: More const-marking cleanup

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-29T08:58:08Z

    Hi,

On Tue, Dec 23, 2025 at 09:47:55PM -0500, Tom Lane wrote:
> Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> > I had another look and it seems to me that the one (src/port/getopt.c) reported
> > and fixed in the attached of the previous email is the only remaining one to fix.
> 
> This fell off my radar for a bit, but pushed now. 

Thanks!

> This does raise a question in my mind though: we did not find this
> case the first time around because machines that have new-enough
> compilers to detect such issues will probably not need our version of
> getopt().  So, what other not-always-compiled bits of code could use
> improvement, and how could we find them without undue effort?  (This
> extends to more issues than just casting-away-const of course.)

That's a fair point. In fact this particular one has been found with the help of
a Coccinelle script, so your remark makes me think of this thread [1]. Indeed,
Coccinelle could help address this by analyzing the code tree without needing to
compile it.

We could imagine:

- adding Coccinelle scripts to the code tree, or
- running BF members with a set of Coccinelle scripts, or
- running a set of Coccinelle scripts at regular intervals

Analyzing the code tree without the need to compile it looks like a valid answer
to your concern, thoughts?

[1]: https://www.postgresql.org/message-id/aQh79jMqU7mq03Hv%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