Why the asprintf patch is still breaking the buildfarm - Mailing list pgsql-hackers

From Tom Lane
Subject Why the asprintf patch is still breaking the buildfarm
Date
Msg-id 10865.1382428694@sss.pgh.pa.us
Whole thread Raw
Responses Re: Why the asprintf patch is still breaking the buildfarm  (David Rowley <dgrowleyml@gmail.com>)
Re: Why the asprintf patch is still breaking the buildfarm  (Manlio Perillo <manlio.perillo@gmail.com>)
List pgsql-hackers
So I returned from vacation only to find that the buildfarm has a bad case
of acne.  All the Windows members are red or pink, and have been for
awhile.  Sigh.

After some research I believe that I understand the reason for the CHECK
failures, at least:

1. src/port/asprintf.c exhibits a truly touching faith that vsnprintf will
report exactly the number of bytes that would have been required, even if
the buffer is not that large.  While this is what is specified in recent
versions of the POSIX standard, older platforms have much sketchier
behavior.

2. In particular, our own src/port/snprintf.c follows the SUS v2 rule that
it should report the number of bytes it *actually wrote*.  This means
that asprintf.c will never think that its initial 128-byte allocation was
insufficient.  So, on platforms where we use this implementation (notably
including Windows), the result of any asprintf call is effectively
truncated at 128 bytes.

3. I believe the exact cause of the reported failures is that the
add_to_path calls in pg_regress.c result in truncating the value of the
PATH environment value, causing system() to not find the "perl"
executable.  (jacana is probably failing because of truncation of
LD_LIBRARY_PATH instead, which is unsurprising since the problem would
move around depending on the directory path lengths in use.)

IMO src/port/asprintf.c is hopelessly naive, as well as ugly and
undocumented.  We should throw it away and replace it with an
implementation more like stringinfo.c's appendStringInfo, which is code
that has been through the wars and is known to be pretty bulletproof these
days.  Aside from the immediate problem, that would allow us to get rid of
the unportable va_copy calls.  (I say they're unportable because no such
functionality is specified in SUS v2.  And no, I do not have any faith at
all in commit c2316dcda1cd057d7d4a56e3a51e3f8f0527e906 as a workaround.)

I have a lot of other gripes about this whole patch, but they can
wait till tomorrow.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Cube extension point support // GSoC'13
Next
From: David Rowley
Date:
Subject: Re: Why the asprintf patch is still breaking the buildfarm