Re: Clean up command argument assembly

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Clean up command argument assembly
Date: 2023-07-05 05:22:33
Message-ID: b555363f-3c56-5761-6f10-e96004a12ad8@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 04.07.23 14:14, Heikki Linnakangas wrote:
> On 26/06/2023 12:33, Peter Eisentraut wrote:
>> This is a small code cleanup patch.
>>
>> Several commands internally assemble command lines to call other
>> commands.  This includes initdb, pg_dumpall, and pg_regress.  (Also
>> pg_ctl, but that is different enough that I didn't consider it here.)
>> This has all evolved a bit organically, with fixed-size buffers, and
>> various optional command-line arguments being injected with
>> confusing-looking code, and the spacing between options handled in
>> inconsistent ways.  This patch cleans all this up a bit to look clearer
>> and be more easily extensible with new arguments and options.
>
> +1

committed

>> We start each command with printfPQExpBuffer(), and then append
>> arguments as necessary with appendPQExpBuffer().  Also standardize on
>> using initPQExpBuffer() over createPQExpBuffer() where possible.
>> pg_regress uses StringInfo instead of PQExpBuffer, but many of the
>> same ideas apply.
>
> It's a bit bogus to use PQExpBuffer for these. If you run out of memory,
> you silently get an empty string instead. StringInfo, which exits the
> process on OOM, would be more appropriate. We have tons of such
> inappropriate uses of PQExpBuffer in all our client programs, though, so
> I don't insist on fixing this particular case right now.

Interesting point. But as you say better dealt with as a separate problem.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-07-05 05:46:42 Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table.
Previous Message Michael Paquier 2023-07-05 05:10:42 Re: Add more sanity checks around callers of changeDependencyFor()