Re: emacs' turn: remove useless if-before-free tests

[Jim Meyering]

Richard M Stallman <address@hidden> wrote:
> Given that most Emacs code calls xfree rather than free, there are not
> many of these if statements, and we rarely add any.
> Why remove them?

Why *not* remove them?

I find that removing the inherent duplication is safe (mechanical),
and makes the code more readable and more maintainable.  For example,

-  if (x_customization_string)
-    free (x_customization_string);
+  free (x_customization_string);

-  if (fdp->infname != NULL) free (fdp->infname);
-  if (fdp->infabsname != NULL) free (fdp->infabsname);
-  if (fdp->infabsdir != NULL) free (fdp->infabsdir);
-  if (fdp->taggedfname != NULL) free (fdp->taggedfname);
-  if (fdp->prop != NULL) free (fdp->prop);
+  free (fdp->infname);
+  free (fdp->infabsname);
+  free (fdp->infabsdir);
+  free (fdp->taggedfname);
+  free (fdp->prop);

Another advantage is that readers won't wonder why the code is performing
what has long been a useless test -- and then re-read to ensure that
the tested expression and the free argument are properly related.
POSIX requires that free(NULL) be a no-op.

The same arguments hold for xfree, once it has been modified to
accept NULL like free does:

-       if (font->bounds.rows[i])
-         xfree (font->bounds.rows[i]);
+       xfree (font->bounds.rows[i]);

-  if (f->output_data.x->saved_menu_event)
-    xfree (f->output_data.x->saved_menu_event);
-
+  xfree (f->output_data.x->saved_menu_event);