Re: Clean up command argument assembly

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

> 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.

--
Heikki Linnakangas
Neon (https://neon.tech)