| From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
|---|---|
| To: | Галкин Сергей <galkin(at)rutoken(dot)ru>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: DEREF_AFTER_NULL: src/common/jsonapi.c:2529 |
| Date: | 2026-04-06 11:59:01 |
| Message-ID: | 107eb23a-8ebb-42bc-99c0-ca551733e94e@proxel.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 4/6/26 10:26 AM, Галкин Сергей wrote:
> That seemed plausible to me, since there is a comment just above saying
> that lex->errormsg can be NULL in shlib code. I also checked
> PQExpBufferBroken(), and it does handle NULL, but that call is under
> #ifdef, while the final access to lex->errormsg->data is unconditional.
>
> I may be missing some invariant here, but it seems worth adding an
> explicit NULL check. I prepared a corresponding patch and am attaching
> it below in case you agree that this is a real issue.
The code is correct but a bit confusing. When JSONAPI_USE_PQEXPBUFFER is
not defined jsonapi_makeStringInfo() will call palloc() which cannot
return NULL so the NULL check (currently done by PQExpBufferBroken()) is
only necessary when JSONAPI_USE_PQEXPBUFFER is defined.
If someone has a patch improving readability I would personally before
merging this since this code feels more complex than it ideally should
be but adding this noop NULL check to silence a false positive from a
static analyzer does not seem like an improvement.
--
Andreas Karlsson
Percona
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andreas Karlsson | 2026-04-06 12:01:41 | Re: DEREF_AFTER_NULL: src/common/jsonapi.c:2529 |
| Previous Message | Amit Kapila | 2026-04-06 11:56:33 | Re: Adding REPACK [concurrently] |