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
signature.asc
Description: PGP signature