On Tue, Mar 12, 2024 at 08:38:43PM -0400, Andrew Dunstan wrote:
> On 2024-03-12 Tu 14:43, Jacob Champion wrote:
>> Two notes that I wanted to point out explicitly:
>> - On the other thread, Daniel contributed a destroyStringInfo()
>> counterpart for makeStringInfo(), which is optional but seemed useful
>> to include here.
>
> yeah, although maybe worth a different patch.
- {
- pfree(lex->strval->data);
- pfree(lex->strval);
- }
+ destroyStringInfo(lex->strval);
I've wanted that a few times, FWIW. I would do a split, mainly for
clarity.
>> - After this patch, if a frontend client calls json_errdetail()
>> without calling freeJsonLexContext(), it will leak memory.
>
> Not too concerned about this:
>
> 1. we tend to be a bit more relaxed about that in frontend programs,
> especially those not expected to run for long times and especially on error
> paths, where in many cases the program will just exit anyway.
>
> 2. the fix is simple where it's needed.
This does not stress me much, either. I can see that OAuth introduces
at least two calls of json_errdetail() in libpq, and that would matter
there.
case JSON_EXPECTED_STRING:
- return psprintf(_("Expected string, but found \"%s\"."),
- extract_token(lex));
+ appendStringInfo(lex->errormsg,
+ _("Expected string, but found \"%.*s\"."),
+ toklen, lex->token_start);
Maybe this could be wrapped in a macro to avoid copying around
token_start and toklen for all the error cases.
--
Michael
