Remove useless if-before-free and if-before-xfree tests

Remove useless if-before-free and if-before-xfree tests

[Jim Meyering]

I removed a bunch of these last year.
Since then, a few have snuck back in, so here's another round.
I've just committed the following two change sets:

From 946385f7a7a6ae08fe37cfeac4e872279a51ce15 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Mon, 29 Jun 2009 07:26:05 +0200
Subject: [PATCH 1/2] Remove useless if-before-free test.

* make-docfile.c (scan_lisp_file): Remove useless test.
---
 lib-src/ChangeLog      |    5 +++++
 lib-src/make-docfile.c |    3 +--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index 2d2956f..590af76 100644
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,8 @@
+2009-06-29  Jim Meyering  <meyering@redhat.com>
+
+	Remove useless if-before-free test.
+	* make-docfile.c (scan_lisp_file): Remove useless test.
+
 2009-06-23  Dan Nicolaescu  <dann@ics.uci.edu>

 	* Makefile.in (movemail.o): Don't pass -Demacs, unused.
diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c
index 219e9d2..ad366bb 100644
--- a/lib-src/make-docfile.c
+++ b/lib-src/make-docfile.c
@@ -907,8 +907,7 @@ scan_lisp_file (filename, mode)
 	      length--;

 	      /* Read in the contents.  */
-	      if (saved_string != 0)
-		free (saved_string);
+	      free (saved_string);
 	      saved_string = (char *) xmalloc (length);
 	      for (i = 0; i < length; i++)
 		saved_string[i] = getc (infile);
--
1.6.3.3.420.gd4b46


From bf3fe8cb2fd3fba21d99ee9fa35a3f07f3485dc7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Mon, 29 Jun 2009 07:26:09 +0200
Subject: [PATCH 2/2] Remove useless if-before-xfree test.

* nsfont.m (nsfont_close): Remove useless test.
* term.c (delete_tty): Likewise.
* w32.c (system_process_attributes): Likewise.
* w32font.c (w32font_close): Likewise.
* xfaces.c (x_free_gc): Likewise.
* xselect.c (buffer): Likewise.
---
 src/ChangeLog |   10 ++++++++++
 src/nsfont.m  |    6 ++----
 src/term.c    |    6 ++----
 src/w32.c     |    3 +--
 src/w32font.c |    3 +--
 src/xfaces.c  |    3 +--
 src/xselect.c |    3 +--
 7 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 010f993..d6099ee 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,13 @@
+2009-06-29  Jim Meyering  <meyering@redhat.com>
+
+	Remove useless if-before-xfree test.
+	* nsfont.m (nsfont_close): Remove useless test.
+	* term.c (delete_tty): Likewise.
+	* w32.c (system_process_attributes): Likewise.
+	* w32font.c (w32font_close): Likewise.
+	* xfaces.c (x_free_gc): Likewise.
+	* xselect.c (buffer): Likewise.
+
 2009-06-28  Andreas Schwab  <schwab@linux-m68k.org>

 	* process.c (send_process): Keep decoded string in a local
diff --git a/src/nsfont.m b/src/nsfont.m
index 68ed1e6..7241af3 100644
--- a/src/nsfont.m
+++ b/src/nsfont.m
@@ -868,10 +868,8 @@ nsfont_close (FRAME_PTR f, struct font *font)

   for (i =0; i<0x100; i++)
     {
-      if (font_info->glyphs[i])
-        xfree (font_info->glyphs[i]);
-      if (font_info->metrics[i])
-        xfree (font_info->metrics[i]);
+      xfree (font_info->glyphs[i]);
+      xfree (font_info->metrics[i]);
     }
   [font_info->nsfont release];
 #ifdef NS_IMPL_COCOA
diff --git a/src/term.c b/src/term.c
index fc77801..5176214 100644
--- a/src/term.c
+++ b/src/term.c
@@ -4018,10 +4018,8 @@ delete_tty (struct terminal *terminal)

   xfree (tty->old_tty);
   xfree (tty->Wcm);
-  if (tty->termcap_strings_buffer)
-    xfree (tty->termcap_strings_buffer);
-  if (tty->termcap_term_buffer)
-    xfree (tty->termcap_term_buffer);
+  xfree (tty->termcap_strings_buffer);
+  xfree (tty->termcap_term_buffer);

   bzero (tty, sizeof (struct tty_display_info));
   xfree (tty);
diff --git a/src/w32.c b/src/w32.c
index cd95f60..23da0ba 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -3995,8 +3995,7 @@ system_process_attributes (pid)
 		}
 	    }
 	}
-      if (buf)
-	xfree (buf);
+      xfree (buf);
     }
   if (!result)
     {
diff --git a/src/w32font.c b/src/w32font.c
index 4148e87..995500a 100644
--- a/src/w32font.c
+++ b/src/w32font.c
@@ -289,8 +289,7 @@ w32font_close (f, font)
     {
       for (i = 0; i < w32_font->n_cache_blocks; i++)
         {
-          if (w32_font->cached_metrics[i])
-            xfree (w32_font->cached_metrics[i]);
+          xfree (w32_font->cached_metrics[i]);
         }
       xfree (w32_font->cached_metrics);
       w32_font->cached_metrics = NULL;
diff --git a/src/xfaces.c b/src/xfaces.c
index 704d7a9..3faaf24 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -785,8 +785,7 @@ x_free_gc (f, gc)
      struct frame *f;
      GC gc;
 {
-  if (gc)
-      xfree (gc);
+  xfree (gc);
 }
 #endif  /* HAVE_NS */

diff --git a/src/xselect.c b/src/xselect.c
index b9b8e0c..5cf4f74 100644
--- a/src/xselect.c
+++ b/src/xselect.c
@@ -2395,8 +2395,7 @@ DEFUN ("x-get-cut-buffer-internal", Fx_get_cut_buffer_internal,

   if (!data || !format)
     {
-      if (data)
-	xfree (data);
+      xfree (data);
       return Qnil;
     }

--
1.6.3.3.420.gd4b46

Re: Remove useless if-before-free and if-before-xfree tests

[Kim F. Storm]

Jim Meyering <jim@meyering.net> writes:

> I removed a bunch of these last year.
> Since then, a few have snuck back in, so here's another round.
> I've just committed the following two change sets:

I haven't looked specifically at the changes, but a test before xfree
may not be useless performance-wise if the normal case is "ptr is NULL".

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Remove useless if-before-free and if-before-xfree tests

[Jim Meyering]

Kim F. Storm wrote:
> Jim Meyering <jim@meyering.net> writes:
>> I removed a bunch of these last year.
>> Since then, a few have snuck back in, so here's another round.
>> I've just committed the following two change sets:
>
> I haven't looked specifically at the changes, but a test before xfree
> may not be useless performance-wise if the normal case is "ptr is NULL".

Hi Kim,

If adding that extra "if" test-before-free leads to a measurable
performance improvement, please let me know.  I've removed hundreds
of such tests in many projects, and so far no one has taken the time
(or been able?) to demonstrate such a thing.

Besides, even if there is a measurable performance difference, in my book,
the cleaner code trumps what can only be a negligible performance hit.

Re: Remove useless if-before-free and if-before-xfree tests

[Stephen J. Turnbull]

Jim Meyering writes:
 > Kim F. Storm wrote:
 > > Jim Meyering <jim@meyering.net> writes:
 > >> I removed a bunch of these last year.
 > >> Since then, a few have snuck back in, so here's another round.
 > >> I've just committed the following two change sets:
 > >
 > > I haven't looked specifically at the changes, but a test before xfree
 > > may not be useless performance-wise if the normal case is "ptr is NULL".

These "normally NULL" cases are likely only in "last chance" cleanup,
which should be outside the loop.

 > Hi Kim,
 > 
 > If adding that extra "if" test-before-free leads to a measurable
 > performance improvement, please let me know.  I've removed hundreds
 > of such tests in many projects, and so far no one has taken the time
 > (or been able?) to demonstrate such a thing.
 > 
 > Besides, even if there is a measurable performance difference, in my book,
 > the cleaner code trumps what can only be a negligible performance hit.

"inline" is your friend.  If you don't trust GCC (and on this kind of
thing, who does? :-), you can force xfree to inline the test by
defining it as a macro.