Обсуждение: pgsql: Remove unnecessary casts in free() and pfree()

Remove unnecessary casts in free() and pfree()

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/45987aae260a441886a010323bf3e143ce8e82d6

Modified Files
--------------
contrib/sepgsql/label.c        | 8 ++++----
src/backend/utils/fmgr/dfmgr.c | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)

On Fri, Aug 26, 2022 at 02:02:33PM +0000, Peter Eisentraut wrote:
> Remove unnecessary casts in free() and pfree()

This seems to be breaking cfbot's "warnings" test.

[07:49:48.983] label.c:665:10: error: passing argument 1 of ‘pfree’ discards ‘const’ qualifier from pointer target type
[-Werror=discarded-qualifiers]
[07:49:48.983]   665 |    pfree(temp);

I imagine it would've been reported earlier, except that this commit was within
60min of the commit that broke perl.

-- 
Justin

Justin Pryzby <pryzby@telsasoft.com> writes:
> On Fri, Aug 26, 2022 at 02:02:33PM +0000, Peter Eisentraut wrote:
>> Remove unnecessary casts in free() and pfree()

> This seems to be breaking cfbot's "warnings" test.

> [07:49:48.983] label.c:665:10: error: passing argument 1 of ‘pfree’ discards ‘const’ qualifier from pointer target
type[-Werror=discarded-qualifiers] 
> [07:49:48.983]   665 |    pfree(temp);

Hmm, well, casting away const is certainly not within pfree's remit,
so I'm glad we changed this.

A quick-n-dirty fix is to cast away const at this call site, but
I wonder whether this isn't a symptom of poor const choices.
I'll take a look later today if no one beats me to it.

            regards, tom lane

I wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
>> This seems to be breaking cfbot's "warnings" test.

> Hmm, well, casting away const is certainly not within pfree's remit,
> so I'm glad we changed this.

Oh, I see: sepgsql's quote_object_name() is doing

    const char *temp;

    temp = quote_identifier(src1);
    appendStringInfoString(&result, temp);
    if (src1 != temp)
        pfree(temp);

evidently because whoever wrote this felt a compulsion to override
quote_identifier's judgment that possibly leaking the quoted identifier
wasn't worth worrying about.  I think we should just nuke this code
altogether and write

    appendStringInfoString(&result, quote_identifier(src1));

            regards, tom lane