[tor-dev] Remove NULL checks for *_free() calls

[Zack Weinberg]

On Sun, Aug 30, 2015 at 10:37 PM, Mansour Moufid
<mansourmoufid at gmail.com> wrote:
> On Sun, Aug 30, 2015 at 8:13 PM, Michael McConville
> <mmcconv1 at sccs.swarthmore.edu> wrote:
>
>> free() is specified to be NULL-safe, and I don't know of any
>> implementations that violate this.
>
> I think those NULL checks are meant to avoid double-free bugs.  If you
> assign NULL to a pointer after you free it and check all pointers
> before free, you avoid trying to free it again.

The thing you may not realize is that free(0) is specified to do
nothing.  This was in the 1989 C standard, so it should be safe to
rely on. I imagine running a Tor relay on SunOS 4.1.x would be a
terrible idea for reasons having nothing to do with the code (e.g.
predictable TCP sequence numbers).

As such, the check is always fully redundant; you get the effect
you're talking about by writing e.g.

    X509_free(x509)
    x509 = 0;

without the if.

> But you did find some places they forgot to assign NULL after free.

Unfortunately, setting pointers to 0 after free doesn't help avoid
double free bugs in practice.  Double frees happen when there are two
different pointers to the same memory block and both holders think
it's their responsibility to deallocate the object.  Clearing one
pointer does precisely nothing to the *other* pointer.

zw