bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

removing useless if-before-free tests


From: Jim Meyering
Subject: removing useless if-before-free tests
Date: Sun, 17 Feb 2008 14:03:35 +0100

I propose to remove tests like this:

        * lib/save-cwd.c (free_cwd): Remove now-useless if-before-free.

diff --git a/lib/save-cwd.c b/lib/save-cwd.c
index 7618f09..e158e8b 100644
--- a/lib/save-cwd.c
+++ b/lib/save-cwd.c
@@ -97,6 +97,5 @@ free_cwd (struct saved_cwd *cwd)
 {
   if (cwd->desc >= 0)
     close (cwd->desc);
-  if (cwd->name)
-    free (cwd->name);
+  free (cwd->name);
 }

To agree is to say that you think "free(NULL)" is always ok, now,
and that gnulib's free module is no longer needed.

Here is some evidence from nearly 1.5 years ago:

    http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html

If anyone can point to a reasonable porting target system for which
we must guard against free(NULL), then please tell us about it.

-----------------
Once you agree to the premise, the next step is to clean up.
We all admit that old habits are hard to break, so I wrote the
/gnulib/build-aux/useless-if-before-free script.  Coreutils uses it in
a new syntax-check rule to ensure that no new useless tests creep back in.

Currently, that script merely detects such useless "if" statements.
However, it contains this suggested code to help you automate their removal:

  git ls-files -z |xargs -0 \
  perl -0x3b -pi -e \
    's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*NULL)?\s*\)\s+(free\s*\(\s*\1\s*\))/$2/s'

Of course not everyone uses git, so this might work better for you:

  find . -name '*.[chly]' -print0 \
    |xargs -0 /gnulib/build-aux/useless-if-before-free -l \
    |xargs -0 perl -0x3b -pi -e \
      's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*NULL)?\s*\)\s+(free\s*\(\s*\1\s*\))/$2/s'

However, processing only .c, .h, .l, and .y files might miss a few.
IMHO, it's better to process all files and examine the result.
I've found matches in .py files that generate C source.

Beware: if you do use the above snippet, note that it can
produce syntactically invalid C code.  That happens when the
affected free statement is immediately followed by an "else".
E.g., it would transform this

  if (x)
    free (x);
  else
    foo ();

into this:

  free (x);
  else
    foo ();

Also note: while the script detects brace-enclosed blocks like
"if (x) { free (x); }", the useless-test-removing code does not.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]