Re: libpq: Remove redundant null pointer checks before free()

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Thu, Jun 16, 2022 at 10:07:33PM +0200, Peter Eisentraut wrote:
>> calls, where the "if" part is unnecessary. This is of course pretty
>> harmless, but some functions like scram_free() and freePGconn() have become
>> so bulky that it becomes annoying. So while I was doing some work in that
>> area I undertook to simplify this.

> Seems fine. Would some of the buildfarm dinosaurs hiccup on that?
> gaur is one that comes into mind.

Doubt it. (In any case, gaur/pademelon are unlikely to be seen
again after a hardware failure --- I'm working on resurrecting that
machine using modern NetBSD on an external drive, but its HPUX
installation probably isn't coming back.)

POSIX has required free(NULL) to be a no-op since at least SUSv2 (1997).
Even back then, the machines that failed on it were legacy devices,
like then-decade-old SunOS versions. So I don't think that Peter's
proposal has any portability risk today.

Having said that, the pattern "if (x) free(x);" is absolutely
ubiquitous across our code, and so I'm not sure that I'm on
board with undoing it only in libpq. I'd be happier if we made
a push to get rid of it everywhere. Notably, I think the choice
that pfree(NULL) is disallowed traces directly to worries about
coding-pattern-compatibility with pre-POSIX free(). Should we
revisit that?

Independently of that concern, how much of a back-patch hazard
might we create with such changes?

regards, tom lane