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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers