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

Michael Paquier <michael@paquier.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