On 06.08.26 02:29, Chao Li wrote:
On Aug 6, 2026, at 03:44, Peter Eisentraut <[email protected]> wrote:
On 05.08.26 05:36, Chao Li wrote:
On Aug 4, 2026, at 17:36, Peter Eisentraut <[email protected]> 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-8 characters (or potentially
analogously in other encodings, but that is not implemented). However, many
callers are not 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 the former 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 of mbvalidate(). 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 same for
printTableAddHeader().
In passing, also simplify the code a bit. There were essentially duplicate
mechanisms for keeping track of the most recently added
cell (fields .cell and .cellsadded). Make that consistent by using an integer counter
for everything. That makes the code 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.
Sorry about that. I was just about to point out what was missing, so I made
those quick-and-dirty changes without reviewing them as carefully as I would
when proposing a patch.
For 0004, it seems to introduce a memory leak in printTableAddCell():
Thanks, I have fixed that in the attached patch.
<v2-0001-Remove-useless-confusing-const-qualifiers.patch><v2-0002-Remove-useless-ENABLE_NLS-conditionals.patch><v2-0003-Use-frontend-logging-API-in-fe_utils-print.c.patch><v2-0004-Make-printTableAddCell-printTableAddHeader-string.patch><v2-0005-Use-frontend-logging-API-in-fe_utils-string_utils.patch>
V2 LGTM.
Committed, thanks.
