I wrote:
> I have a lot of other gripes about this whole patch, but they can
> wait till tomorrow.
The other big thing I don't like about this patch is that adopting
asprintf() as a model was not any better thought-out than the
portability considerations were. It's a bad choice because:
1. asprintf() is not in C99 nor any POSIX standard.
2. The two known implementations, glibc and *BSD, behave differently,
as admitted in the Linux man page. Thus, there is nothing resembling
a standard here.
3. Neither implementation has given any thought to error reporting;
it's not possible to tell whether a failure was due to out-of-memory
or a vsnprintf failure such as a format-string error. (pg_asprintf
assumes the former, which probably has something to do with the bogus
out-of-memory failures we're seeing on buildfarm member "anole", though
I'm not sure about the details.)
4. The use of a char** output parameter is not very friendly, because
(at least on older versions of gcc) it disables uninitialized-variable
detection. That is, if you have code like
char **str;
if (...) asprintf(&str, ...);else asprintf(&str, ...);... use str for something ...
many compilers will not be able to check whether you actually assigned
to str in both code paths.
5. The reason for the char** parameter is that the function return
value is needed for error/buffer overrun handling --- but no caller
of pg_asprintf looks at the return value at all.
6. Thus, I much prefer the API design that was used for psprintf,
that is return the allocated buffer pointer as the function result.
It seems like a bad idea to me that we adopted two different APIs
for frontend and backend code; if we're going to hardwire
exit()-on-failure into pg_asprintf there is no benefit whatsoever
to reserving the function return value for failure indications.
In short, I want to change pg_asprintf to return the malloc'd buffer
as its function result. This probably means we should change the
function name too, since the allusion to asprintf is completely useless
if we do that. I'm thinking pg_psprintf for the frontend version of
psprintf, but maybe somebody has a better suggestion?
regards, tom lane
