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.