MIME-Version: 1.0
References: <0b32e30f2fb94ae3b7f4ee15bbb072c0@rutoken.ru>
 <107eb23a-8ebb-42bc-99c0-ca551733e94e@proxel.se>
 <CAOYmi+muy=Wtk0dv6J5HmcFTMvrPMe2MGN2X+hjZL7DKSUEXLQ@mail.gmail.com>
 <leyiwkml7kyrdxwjzhcmmi6rtyxxwofokfxhlkpbagfprmbqw2@jxdckvomcmug>
 <CAOYmi+mneT-m52GhWdA=XqH5SE6f5jJbAmjwL+wp+xk4j9VVpg@mail.gmail.com>
 <mazv3o3ua5pbvy5xwrz6n6zu6gyxkqtohbdohu3o73xyhtwa26@xcbb7ro2o42k>
In-Reply-To: <mazv3o3ua5pbvy5xwrz6n6zu6gyxkqtohbdohu3o73xyhtwa26@xcbb7ro2o42k>
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Mon, 6 Apr 2026 15:01:14 -0700
Message-ID: 
 <CAOYmi+k__DbMjOxfySjVjCQB=-7Pd+qHb-1Vn9kuN1XOgbOsGg@mail.gmail.com>
Subject: Re: DEREF_AFTER_NULL: src/common/jsonapi.c:2529
To: Andres Freund <andres@anarazel.de>
Cc: Andreas Karlsson <andreas@proxel.se>,
 =?UTF-8?B?0JPQsNC70LrQuNC9INCh0LXRgNCz0LXQuQ==?= <galkin@rutoken.ru>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAOYmi%2Bk__DbMjOxfySjVjCQB%3D-7Pd%2BqHb-1Vn9kuN1XOgbOsGg%40mail.gmail.com>
Precedence: bulk

On Mon, Apr 6, 2026 at 2:40=E2=80=AFPM Andres Freund <andres@anarazel.de> w=
rote:
> > Maybe I didn't parse this question correctly, but I don't want to
> > avoid freeing NULLs. It's entirely reasonable and normal to write code
> > that frees NULLs.
>
> I think it's a bad idea ever call free(), realloc() etc with a NULL.  It'=
s imo
> quite the code smell indicating that code lost of track of whether someth=
ing
> was allocated or not.

I could not disagree more strongly:
- the alternative for many developers in practice is going to be a
unilateral `if (ptr) free(ptr);` anyway, losing any potential
"benefit", and
- I'm not even convinced that "lose track of whether something was
allocated" is a thing. If it was NULL, it was not allocated. If it is
not NULL, it is either allocated, or you're about to double-free
something, which has nothing to do with free(NULL). What's to "lose
track" of?

> The whole point of having pfree() in FE code is to make it possible to wr=
ite
> common code that doesn't need ifdef around every allocation.

Which didn't work out for us, in my humble opinion, as soon as libpq
entered the equation. We don't have to name the abstraction layer the
same thing as only one of the branches of the abstraction.

> > Or else make pfree() behave like free() [2] so that we don't
> > have to have that particular papercut at all anymore?
>
> -many
>
> It's also not a path I want to add any unnecessary instructions to.

Okay, but I'd be curious to know how widespread this position is.

> > It still doesn't help the OOM abstraction leak between libpq and the
> > backend, as far as I can tell.
>
> If the code were to use a JsonLexContext field to decide whether to pass
> MCXT_ALLOC_NO_OOM to the allocation functions etc it'd at least would mak=
e the
> code more similar between FE/BE due to both having to deal with NULLs.

I'm talking about libpq here; we don't link fe_memutils.c at all.

--Jacob