Bug 839925 - avoid useless if-before-free (or free-like) function call
avoid useless if-before-free (or free-like) function call
Status: CLOSED CURRENTRELEASE
Product: GlusterFS
Classification: Community
Component: build (Show other bugs)
3.3.0
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Amar Tumballi
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-13 04:43 EDT by Jim Meyering
Modified: 2013-12-18 19:08 EST (History)
7 users (show)

See Also:
Fixed In Version: glusterfs-3.4.0
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-07-24 13:35:41 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jim Meyering 2012-07-13 04:43:11 EDT
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 16:56:39 EDT
CHANGE: http://review.gluster.com/3651 (no semantic change: reverse condition and if/else clauses...) merged in master by Anand Avati (avati@redhat.com)
Comment 2 Vijay Bellur 2012-07-13 17:03:51 EDT
CHANGE: http://review.gluster.com/3661 (remove useless if-before-free (and free-like) functions) merged in master by Anand Avati (avati@redhat.com)

Note You need to log in before you can comment on or make changes to this bug.