Date: Tue, 10 Mar 2020 12:39:53 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>,
	Kuntal Ghosh <kuntalghosh.2007@gmail.com>,
	Alexey Kondratov <a.kondratov@postgrespro.ru>,
	Liudmila Mantrova <l.mantrova@postgrespro.ru>,
	Thomas Munro <thomas.munro@gmail.com>,
	Andrey Borodin <x4mmm@yandex-team.ru>,
	David Steele <david@pgmasters.net>,
	Andres Freund <andres@anarazel.de>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Dmitry Dolgov <9erthalion6@gmail.com>, vladimirlesk@yandex-team.ru,
	dsarafan@yandex-team.ru
Subject: Re: [Patch] pg_rewind: options to use restore_command from
 recovery.conf or command line
Message-ID: <20200310153953.GA14468@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200310124358.GB1538@paquier.xyz>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Mar-10, Michael Paquier wrote:

> On Tue, Mar 10, 2020 at 01:05:40PM +0300, Alexander Korotkov wrote:
> > Two options seem reasonable to me in this case.  The first is to pass
> > length as additional argument as you did.  The second option is to
> > make argument a pointer to fixed-size array as following.

Another option is to return the command as a palloc'ed string (per
psprintf), instead of using a caller-stack-allocated variable.  Passing
the buffer len is widely used, but more error prone (and I think getting
this one wrong might be more catastrophic than a mistake elsewhere.)
This is not a performance-critical path enough that we *need* the
optimization that avoids the palloc is important.  (Failure can be
reported by returning NULL.)  Also, I think the function comment could
stand some more detailing.

Also, I think Msvcbuild.pm could follow Makefile's ideas of one line per
file.  Maybe no need to fix all of that in this patch, but let's start
by adding the new file it its own line rather than reflowing two
adjacent lines (oh wait ... does perltidy put it that way?  if so,
nevermind.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services