Re: [PATCH] Make jsonapi usable from libpq

2021-07-07 Thread Jacob Champion
On Wed, 2021-07-07 at 01:42 -0400, Tom Lane wrote: > Michael Paquier writes: > > It seems to me that this does not address yet the problems with the > > palloc/pstrdup in jsonapi.c though, which would need to rely on > > malloc() if we finish to use this code in libpq. I am not sure yet > > that

Re: [PATCH] Make jsonapi usable from libpq

2021-07-06 Thread Tom Lane
Michael Paquier writes: > It seems to me that this does not address yet the problems with the > palloc/pstrdup in jsonapi.c though, which would need to rely on > malloc() if we finish to use this code in libpq. I am not sure yet > that we have any need to do that yet as we may finish by not

Re: [PATCH] Make jsonapi usable from libpq

2021-07-06 Thread Michael Paquier
On Tue, Jun 29, 2021 at 03:34:29PM -0400, Tom Lane wrote: > Actually, I'd forgotten that the PQExpBuffer functions are already > exported by libpq, and much of our frontend code already uses them > from there. So we don't really need to move anything unless there's > a call to use this code in

Re: [PATCH] Make jsonapi usable from libpq

2021-06-30 Thread Tom Lane
Peter Eisentraut writes: > On 26.06.21 19:43, Tom Lane wrote: >> fe-print.c's handling of OOM isn't nice at all: >> fprintf(stderr, libpq_gettext("out of memory\n")); >> abort(); >> Although fe-print.c is semi-deprecated, it still seems like it'd >> be a good idea to clean that up. >

Re: [PATCH] Make jsonapi usable from libpq

2021-06-30 Thread Peter Eisentraut
On 26.06.21 19:43, Tom Lane wrote: I spent some time looking for other undesirable symbol dependencies in libpq, and soon found a couple. PGTHREAD_ERROR potentially calls abort(), which seems even worse than exit-on-OOM, although I don't think we've ever heard a report of that being hit. Also,

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Tom Lane
Jacob Champion writes: > On Tue, 2021-06-29 at 14:50 -0400, Tom Lane wrote: >> The existing convention is to use pqexpbuffer.c, which seems strictly >> cleaner and more robust than asprintf. In particular its behavior under >> OOM conditions is far easier/safer to work with. Maybe we should

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Jacob Champion
On Tue, 2021-06-29 at 14:50 -0400, Tom Lane wrote: > Jacob Champion writes: > > What would you think about a src/port of asprintf()? Maybe libpq > > doesn't change quickly enough to worry about it, but having developers > > revisit stack allocation for strings every time they target the libpq > >

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Tom Lane
Jacob Champion writes: > What would you think about a src/port of asprintf()? Maybe libpq > doesn't change quickly enough to worry about it, but having developers > revisit stack allocation for strings every time they target the libpq > parts of the code seems like a recipe for security problems.

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Jacob Champion
On Sat, 2021-06-26 at 09:36 +0900, Michael Paquier wrote: > On Fri, Jun 25, 2021 at 08:58:46PM +, Jacob Champion wrote: > > 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

Re: [PATCH] Make jsonapi usable from libpq

2021-06-29 Thread Jacob Champion
On Sun, 2021-06-27 at 10:43 +0900, Michael Paquier wrote: > On Sat, Jun 26, 2021 at 01:43:50PM -0400, Tom Lane wrote: > > BTW, so far as the original topic of this thread is concerned, > > it sounds like Jacob's ultimate goal is to put some functionality > > into libpq that requires JSON parsing.

Re: [PATCH] Make jsonapi usable from libpq

2021-06-28 Thread Daniel Gustafsson
> On 28 Jun 2021, at 21:15, Tom Lane wrote: > I wrote: >>> Not real sure what to do about PGTHREAD_ERROR. > >> I wonder if we shouldn't simply nuke that macro and change the >> call sites into "Assert(false)". > > After further study this still seems like the best available choice. While this

Re: [PATCH] Make jsonapi usable from libpq

2021-06-28 Thread Tom Lane
I wrote: >> Not real sure what to do about PGTHREAD_ERROR. > I wonder if we shouldn't simply nuke that macro and change the > call sites into "Assert(false)". After further study this still seems like the best available choice. We do not have the option to make either default_threadlock() or

Re: [PATCH] Make jsonapi usable from libpq

2021-06-26 Thread Michael Paquier
On Sat, Jun 26, 2021 at 01:43:50PM -0400, Tom Lane wrote: > BTW, so far as the original topic of this thread is concerned, > it sounds like Jacob's ultimate goal is to put some functionality > into libpq that requires JSON parsing. I'm going to say up front > that that sounds like a terrible

Re: [PATCH] Make jsonapi usable from libpq

2021-06-26 Thread Tom Lane
I wrote: > Not real sure what to do about PGTHREAD_ERROR. I wonder if we shouldn't simply nuke that macro and change the call sites into "Assert(false)". The only call sites are in default_threadlock() (used in fe_auth.c) and pq_lockingcallback() (for OpenSSL). I suggest that 1.

Re: [PATCH] Make jsonapi usable from libpq

2021-06-26 Thread Tom Lane
I wrote: > I spent some time looking for other undesirable symbol dependencies > in libpq, and soon found a couple. PGTHREAD_ERROR potentially calls > abort(), which seems even worse than exit-on-OOM, although I don't > think we've ever heard a report of that being hit. Also, > fe-print.c's

Re: [PATCH] Make jsonapi usable from libpq

2021-06-26 Thread Tom Lane
Michael Paquier writes: > On Fri, Jun 25, 2021 at 08:58:46PM +, Jacob Champion wrote: >> 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

Re: [PATCH] Make jsonapi usable from libpq

2021-06-26 Thread Daniel Gustafsson
> On 26 Jun 2021, at 02:36, Michael Paquier wrote: > The service file parsing is the only piece in libpq using StringInfoData. > @Tom, @Daniel, you got involved in c0cb87f. It looks like this piece about > the > limitations with service file parsing needs a rework. This code is new in 14, >

Re: [PATCH] Make jsonapi usable from libpq

2021-06-25 Thread Michael Paquier
On Fri, Jun 25, 2021 at 08:58:46PM +, Jacob Champion wrote: > 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

Re: [PATCH] Make jsonapi usable from libpq

2021-06-25 Thread Jacob Champion
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

Re: [PATCH] Make jsonapi usable from libpq

2021-06-23 Thread Michael Paquier
On Tue, Jun 22, 2021 at 10:59:37PM +, Jacob Champion wrote: > Hmm. I'm honestly hesitant to start splitting files apart, mostly > because json_log_and_abort() is only called from two places, and both > those places are triggered by programmer error as opposed to user > error. > > Would it

[PATCH] Make jsonapi usable from libpq

2021-06-22 Thread Jacob Champion
(This is split off from my work on OAUTHBEARER [1].) The jsonapi in src/common can't currently be compiled into libpq. The first patch here removes the dependency on pg_log_fatal(), which is not available to the sharedlib. The second patch makes sure that all of the return values from