Bug 839925

Summary: avoid useless if-before-free (or free-like) function call
Product: [Community] GlusterFS Reporter: Jim Meyering <meyering>
Component: buildAssignee: Amar Tumballi <amarts>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 3.3.0CC: gluster-bugs, joe, jonathansteffan, matthias, ndevos, silas, vraman
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: glusterfs-3.4.0 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-07-24 17:35:41 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jim Meyering 2012-07-13 08:43:11 UTC 
Description of problem: free(NULL) works fine, so any test whose sole
purpose is to guard against that is wasted code.  Remove such unwanted tests.
E.g., change this

  if (p != NULL)
      free (p);

to this:

  free (p);

Do the same with other free-like constructs like GF_FREE and FREE.
The above change does not make a big difference, but then the
freed expression is long or complicated, it's not as easy to read
(requires visual inspection to see whether the freed expression is
the same as the one tested by the preceding "if").

Running this shows that there are over 700 such uses:
  git grep -i -E -l -z "free *\("|xargs -0 useless-if-before-free --name=free --name=FREE --name=GF_FREE|grep -ic free

Using the useless-if-before-free script from gnulib,
  http://git.sv.gnu.org/cgit/gnulib.git/tree/build-aux/useless-if-before-free

[and per comments at its end]
run it like this to remove the unwanted "if" tests:

for free in free FREE GF_FREE; do
  git grep -l -z "$free *(" \
    | xargs -0 useless-if-before-free -l --name="$free" \
    | xargs -0 perl -0x3b -pi -e \
     's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*(?:0|NULL))?\s*\)\s+('"$free"'\s*\((?:\s*\([^)]+\))?\s*\1\s*\)\s*;)/$2/s'

  git grep -l -z "$free *(" \
    | xargs -0 useless-if-before-free -l --name="$free" \
    | xargs -0 perl -0777 -pi -e \
       's/\bif\s*\(\s*(\S+?)(?:\s*!=\s*(?:0|NULL))?\s*\)\s*\{\s*('"$free"'\s*\((?:\s*\([^)]+\))?\s*\1\s*\);)\s*\}[^\n]*$/$2/gms'
done

However, that induced one error:
in xlators/mgmt/glusterd/src/glusterd-mountbroker.c
it would have removed a seemingly useless "if", but
that if had a matching "else", so cannot be naively removed.

Instead, rewrite the "if" expression in a separate change set,
(so that our script no longer spots it) before running the above.
Comment 1 Vijay Bellur 2012-07-13 20:56:39 UTC 
CHANGE: http://review.gluster.com/3651 (no semantic change: reverse condition and if/else clauses...) merged in master by Anand Avati (avati)
Comment 2 Vijay Bellur 2012-07-13 21:03:51 UTC 
CHANGE: http://review.gluster.com/3661 (remove useless if-before-free (and free-like) functions) merged in master by Anand Avati (avati)