Re: [PATCH] Make pg_basebackup configure and start standby [Review] - Mailing list pgsql-hackers

2012-11-20 20:32 keltezéssel, Boszormenyi Zoltan írta:
> 2012-11-20 17:03 keltezéssel, Boszormenyi Zoltan írta:
>> 2012-11-18 17:20 keltezéssel, Magnus Hagander írta:
>>
>>> Much of the tar stuff is very similar (I haven't looked to see if it's
>>> identical) to the stuff in backend/replication/basebackup.c. Perhaps
>>> we should have a src/port/tarutil.c?
>>
>> I will implement it as a separate patch.
>
> I implemented it, all 3 patches are attached now. Use this patchset,
> the previously sent 1st patch had a bug, it called conninit_storeval()
> with value == NULL arguments and it crashes on strdup(NULL).
>
>>
>>> escape_string() - already exists as escape_quotes() in initdb, AFAICT.
>>> We should either move it to src/port/, or at least copy it so it's
>>> exactly the same.
>>
>> A copy of escape_quotes() is now in pg_basebackup.c and is used.
>>
>> I will also unify the copies of it in a separate patch.
>
> This one is not done yet, but a suggestion on which existing file
> it can fit into or for a new src/port/filename is welcome.

I experimented with unifying escape_quotes() shortly.

The problem is that it calls pg_malloc() which is an executable-specific
function. Some of the bin/* executables define it as calling exit(1)
when malloc() returns NULL, some call it with exit(EXIT_FAILURE)
which happens to be 1 but still can be different from the constant 1.
Some executables only define pg_malloc0() but not pg_malloc().

pg_basebackup needs pg_malloc() to call disconnect_and_exit(1)
instead to quit cleanly and not leave an "unexpected EOF from client"
message in the server log. Which is a macro at the moment, but
has to be turned into a real function for the reasons below.

To unify escape_quotes(), pg_malloc() need to be also unified.
But the diverse requirements for pg_malloc() from different
executables means that both the escape_quotes() and the pg_malloc()
interface need to be extended with the exit function they have to call
and the argument to pass to the exit function. Unless we don't care
about the bug reports about "unexpected EOF from client" messages.

Frankly, it doesn't worth the effort. Let's just keep the two separate
copies of escape_quotes().

BTW, it seems to me that this "unify even the least used functions"
mentality was not considered to be a great feature during the
introduction of pg_malloc(), which is used in far more code than
the TAR functions.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de     http://www.postgresql.at/