Re: [PATCH] Make jsonapi usable from libpq

On Sat, 2021-06-26 at 09:36 +0900, Michael Paquier wrote:
> On Fri, Jun 25, 2021 at 08:58:46PM +0000, 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 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.
> 
> We could do this cleanup first, as an independent patch.  That's
> simple enough.  I am wondering if we'd better do this bit in 14
> actually, so as the divergence between 15~ and 14 is lightly
> minimized.

Up to you in the end; I don't have a good intuition for whether the
code motion would be worth it for 14, if it's not actively used.

> > > 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.

This seems to have spawned an entirely new thread over the weekend,
which I will watch with interest. :)

> > 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.
> 
> Yeah, a side effect of that is to enforce a new rule for any frontend
> code that calls palloc(), and these could be easily exposed to crashes
> within knowing about it until their system is under resource
> pressure.  Silent breakages with very old guaranteed behaviors is
> bad.

Fair point.

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.

--Jacob