From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, alvherre(at)alvh(dot)no-ip(dot)org |
Subject: | Re: Support json_errdetail in FRONTEND builds |
Date: | 2024-03-13 06:37:47 |
Message-ID: | ZfFJuwppCFs5awP6@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2024-03-13 06:42:31 | Re: meson vs tarballs |
Previous Message | Andrew Dunstan | 2024-03-13 06:31:46 | Re: meson vs tarballs |