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

[Michael McConville]

Mansour Moufid wrote:
> Michael McConville 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.
> 
> Like there:
> 
> >   error:
> > -  if (x509) {
> > -    X509_free(x509);
> > -    x509 = NULL;
> > -  }

I don't see how this could ever produce functionally different code. Can
you give an example?

The above block only runs if x509 != NULL. My patch removes the
condition, making it also run when x509 == NULL. But that just frees
NULL (effectively a nop) and then reassigns x509 to NULL...

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

For one reason or another, this isn't common practice. Do the Tor style
guidelines suggest that it should always be done? I only saw it rarely
in the codebase.