Re: Make printTableAddCell/printTableAddHeader string argument const - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: Make printTableAddCell/printTableAddHeader string argument const
Date
Msg-id bc4413ff-fa91-4850-aa26-cf9e60cd19dc@eisentraut.org
Whole thread
In response to Re: Make printTableAddCell/printTableAddHeader string argument const  (Chao Li <li.evan.chao@gmail.com>)
Responses Re: Prevent remote libpq notices from being sent to clients
List pgsql-hackers
On 05.08.26 05:36, Chao Li wrote:
> 
> 
>> On Aug 4, 2026, at 17:36, Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>> These functions are used in psql to assemble tables to print.
>>
>> They would sometimes overwrite the string argument they are passed, namely via mbvalidate(), which removes invalid
UTF-8characters (or potentially analogously in other encodings, but that is not implemented).  However, many callers
arenot expecting that.  In many callers, the input value comes directly from libpq structures, such as from
PQgetvalue()or PQsslAttribute().  The latter actually has a const char * return type, and that was cast away.  But even
theformer is not expecting its return value to be modified.
 
>>
>> Fix that by making these arguments const.  Internally, we add a separate function that does only the checking part
ofmbvalidate().  Only if the validation returns a negative result, we make a copy and run mbvalidate() on the copy.
printTableAddCell()already had internal infrastructure for keeping track of what values needed to be freed.  We add the
samefor printTableAddHeader().
 
>>
>> In passing, also simplify the code a bit.  There were essentially duplicate mechanisms for keeping track of the most
recentlyadded
 
>> cell (fields .cell and .cellsadded).  Make that consistent by using an integer counter for everything.  That makes
thecode arguably easier to read than with the "current pointer" approaches.
 
>>
>> The first three patches are preparation patches to further clean up the nearby code a bit.
>>
<0001-Remove-useless-confusing-const-qualifiers.patch><0002-Remove-useless-ENABLE_NLS-conditionals.patch><0003-Use-frontend-logging-API-in-fe_utils-print.c.patch><0004-Make-printTableAddCell-printTableAddHeader-string-ar.patch>
> 
> 0001, 0002 and 0003 look good to me.
> 
> For 0001 and 0003, I searched over the source tree, and found a few more occurrences, see the attached diff files.

Ok, I added these.  (I added your 0001 to my 0001 and added your 0003 as 
a separate patch.)

Note that your 0001 was incomplete: It did not update the "const bool 
newline" in the add_tablespace_footer() definition.

Also, in your 0003, the messages still contained newlines, but the 
logging API adds its own newlines and in fact rejects strings that end 
with newlines, so that patch wouldn't have worked.  I have fixed that.

> For 0004, it seems to introduce a memory leak in printTableAddCell():

Thanks, I have fixed that in the attached patch.

Attachment

pgsql-hackers by date:

Previous
From: Matthias van de Meent
Date:
Subject: Re: [PATCH] Rebuild CHECK constraints after generated column SET EXPRESSION
Next
From: Mark Atwood
Date:
Subject: Use EVP_MAC for HMAC with OpenSSL 3.0 and later