On Thu, 2021-06-24 at 14:56 +0900, Michael Paquier wrote: > Looking more closely at that, I actually find a bit crazy the > requirement for any logging within jsonapi.c just to cope with the > fact that json_errdetail() and report_parse_error() just want to track > down if the caller is giving some garbage or not, which should never > be the case, really. So I would be tempted to eliminate this > dependency to begin with.
I think that's a good plan. > The second thing is how we should try to handle the way the error > message gets allocated in json_errdetail(). libpq cannot rely on > psprintf(), That surprised me. So there's currently no compiler-enforced prohibition, just a policy? It looks like the bar was lowered a little bit in commit c0cb87fbb6, as libpq currently has a symbol dependency on pg_get_line_buf() and pfree() on my machine. > , so I can think about two options here: > - Let the caller of json_errdetail() allocate the memory area of the > error message by itself, pass it down to the function. > - Do the allocation within json_errdetail(), and let callers cope the > case where json_errdetail() itself fails on OOM for any frontend code > using it. > > Looking at HEAD, the OAUTH patch and the token handling, the second > option looks more interesting. I have to admit that handling the > token part makes the patch a bit special, but that avoids duplicating > all those error messages for libpq. Please see the idea as attached. I prefer the second approach as well. Looking at the sample implementation -- has an allocating sprintf() for libpq really not been implemented before? Doing it ourselves on the stack gives me some heartburn; at the very least we'll have to make careful use of snprintf so as to not smash the stack while parsing malicious JSON. If our libpq-specific implementation is going to end up returning NULL on bad allocation anyway, could we just modify the behavior of the existing front-end palloc implementation to not exit() from inside libpq? That would save a lot of one-off code for future implementers. --Jacob
