From 8e0f70033b2bd1679a6e5971978fdc3ee09bdb72 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 31 Jan 2008 18:26:32 +0100 Subject: [PATCH] Avoid unnecessary "if-before-free" tests. This change removes all obvious useless if-before-free tests. E.g., it replaces code like this: if (some_expression) free (some_expression); with the now-equivalent: free (some_expression); It is equivalent not just because POSIX has required free(NULL) to work for a long time, but simply because it has worked for so long that no reasonable porting target fails the test. Here's some evidence from nearly 1.5 years ago: http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html FYI, the change below was prepared by running the following: 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' Note however, that it doesn't handle brace-enclosed blocks like "if (x) { free (x); }". But that's ok, since there were none like that in git sources. Beware: if you do use the above snippet, note that it can produce syntactically invalid C code. That happens when the affected "if"-statement has a matching "else". E.g., it would transform this if (x) free (x); else foo (); into this: free (x); else foo (); There were none of those here, either. If you're interested in automating detection of the useless tests, you might like the useless-if-before-free script in gnulib: [it *does* detect brace-enclosed free statements, and has a --name=S option to make it detect free-like functions with different names] http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=build-aux/useless-if-before-free Addendum: Remove one more (in imap-send.c), spotted by Jean-Luc Herren . Signed-off-by: Jim Meyering Signed-off-by: Junio C Hamano --- builtin-blame.c | 3 +-- builtin-branch.c | 9 +++------ builtin-fast-export.c | 3 +-- builtin-http-fetch.c | 3 +-- builtin-pack-objects.c | 3 +-- builtin-revert.c | 3 +-- connect.c | 3 +-- diff.c | 9 +++------ dir.c | 3 +-- http-push.c | 18 ++++++------------ imap-send.c | 5 ++--- interpolate.c | 3 +-- pretty.c | 3 +-- remote.c | 3 +-- setup.c | 3 +-- sha1_name.c | 6 ++---- xdiff-interface.c | 3 +-- 17 files changed, 28 insertions(+), 55 deletions(-) diff --git a/builtin-blame.c b/builtin-blame.c index 59d7237f21..bfd562d7d2 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -123,8 +123,7 @@ static inline struct origin *origin_incref(struct origin *o) static void origin_decref(struct origin *o) { if (o && --o->refcnt <= 0) { - if (o->file.ptr) - free(o->file.ptr); + free(o->file.ptr); free(o); } } diff --git a/builtin-branch.c b/builtin-branch.c index 9edf2eb816..79177007e6 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -126,8 +126,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) continue; } - if (name) - free(name); + free(name); name = xstrdup(mkpath(fmt, argv[i])); if (!resolve_ref(name, sha1, 1, NULL)) { @@ -172,8 +171,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds) } } - if (name) - free(name); + free(name); return(ret); } @@ -490,8 +488,7 @@ static void create_branch(const char *name, const char *start_name, if (write_ref_sha1(lock, sha1, msg) < 0) die("Failed to write ref: %s.", strerror(errno)); - if (real_ref) - free(real_ref); + free(real_ref); } static void rename_branch(const char *oldname, const char *newname, int force) diff --git a/builtin-fast-export.c b/builtin-fast-export.c index f741df5220..49b54de054 100755 --- a/builtin-fast-export.c +++ b/builtin-fast-export.c @@ -196,8 +196,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev) ? strlen(reencoded) : message ? strlen(message) : 0), reencoded ? reencoded : message ? message : ""); - if (reencoded) - free(reencoded); + free(reencoded); for (i = 0, p = commit->parents; p; p = p->next) { int mark = get_object_mark(&p->item->object); diff --git a/builtin-http-fetch.c b/builtin-http-fetch.c index 7f450c61d9..299093ff91 100644 --- a/builtin-http-fetch.c +++ b/builtin-http-fetch.c @@ -80,8 +80,7 @@ int cmd_http_fetch(int argc, const char **argv, const char *prefix) walker_free(walker); - if (rewritten_url) - free(rewritten_url); + free(rewritten_url); return rc; } diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index d2bb12e574..7dff6536de 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1428,8 +1428,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src, * accounting lock. Compiler will optimize the strangeness * away when THREADED_DELTA_SEARCH is not defined. */ - if (trg_entry->delta_data) - free(trg_entry->delta_data); + free(trg_entry->delta_data); cache_lock(); if (trg_entry->delta_data) { delta_cache_size -= trg_entry->delta_size; diff --git a/builtin-revert.c b/builtin-revert.c index e219859f9b..b6dee6a56c 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -397,8 +397,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) else return execl_git_cmd("commit", "-n", "-F", defmsg, NULL); } - if (reencoded_message) - free(reencoded_message); + free(reencoded_message); return 0; } diff --git a/connect.c b/connect.c index 5ac3572784..d12b105970 100644 --- a/connect.c +++ b/connect.c @@ -68,8 +68,7 @@ struct ref **get_remote_heads(int in, struct ref **list, name_len = strlen(name); if (len != name_len + 41) { - if (server_capabilities) - free(server_capabilities); + free(server_capabilities); server_capabilities = xstrdup(name + name_len + 1); } diff --git a/diff.c b/diff.c index 699b21f4e3..f08e663b86 100644 --- a/diff.c +++ b/diff.c @@ -118,8 +118,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v pp->next = funcname_pattern_list; funcname_pattern_list = pp; } - if (pp->pattern) - free(pp->pattern); + free(pp->pattern); pp->pattern = xstrdup(value); return 0; } @@ -492,10 +491,8 @@ static void free_diff_words_data(struct emit_callback *ecbdata) ecbdata->diff_words->plus.text.size) diff_words_show(ecbdata->diff_words); - if (ecbdata->diff_words->minus.text.ptr) - free (ecbdata->diff_words->minus.text.ptr); - if (ecbdata->diff_words->plus.text.ptr) - free (ecbdata->diff_words->plus.text.ptr); + free (ecbdata->diff_words->minus.text.ptr); + free (ecbdata->diff_words->plus.text.ptr); free(ecbdata->diff_words); ecbdata->diff_words = NULL; } diff --git a/dir.c b/dir.c index 1f507daff2..edc458e020 100644 --- a/dir.c +++ b/dir.c @@ -704,8 +704,7 @@ static struct path_simplify *create_simplify(const char **pathspec) static void free_simplify(struct path_simplify *simplify) { - if (simplify) - free(simplify); + free(simplify); } int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec) diff --git a/http-push.c b/http-push.c index 0beb7406c3..406270f6f4 100644 --- a/http-push.c +++ b/http-push.c @@ -664,8 +664,7 @@ static void release_request(struct transfer_request *request) close(request->local_fileno); if (request->local_stream) fclose(request->local_stream); - if (request->url != NULL) - free(request->url); + free(request->url); free(request); } @@ -1283,10 +1282,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout) strbuf_release(&in_buffer); if (lock->token == NULL || lock->timeout <= 0) { - if (lock->token != NULL) - free(lock->token); - if (lock->owner != NULL) - free(lock->owner); + free(lock->token); + free(lock->owner); free(url); free(lock); lock = NULL; @@ -1344,8 +1341,7 @@ static int unlock_remote(struct remote_lock *lock) prev->next = prev->next->next; } - if (lock->owner != NULL) - free(lock->owner); + free(lock->owner); free(lock->url); free(lock->token); free(lock); @@ -2035,8 +2031,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1) } free(url); - if (*symref != NULL) - free(*symref); + free(*symref); *symref = NULL; hashclr(sha1); @@ -2435,8 +2430,7 @@ int main(int argc, char **argv) } cleanup: - if (rewritten_url) - free(rewritten_url); + free(rewritten_url); if (info_ref_lock) unlock_remote(info_ref_lock); free(remote); diff --git a/imap-send.c b/imap-send.c index 9025d9aa3e..10cce15a42 100644 --- a/imap-send.c +++ b/imap-send.c @@ -472,7 +472,7 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb, if (socket_write( &imap->buf.sock, buf, bufl ) != bufl) { free( cmd->cmd ); free( cmd ); - if (cb && cb->data) + if (cb) free( cb->data ); return NULL; } @@ -858,8 +858,7 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd ) normal: if (cmdp->cb.done) cmdp->cb.done( ctx, cmdp, resp ); - if (cmdp->cb.data) - free( cmdp->cb.data ); + free( cmdp->cb.data ); free( cmdp->cmd ); free( cmdp ); if (!tcmd || tcmd == cmdp) diff --git a/interpolate.c b/interpolate.c index 6ef53f2465..7f03bd99c5 100644 --- a/interpolate.c +++ b/interpolate.c @@ -11,8 +11,7 @@ void interp_set_entry(struct interp *table, int slot, const char *value) char *oldval = table[slot].value; char *newval = NULL; - if (oldval) - free(oldval); + free(oldval); if (value) newval = xstrdup(value); diff --git a/pretty.c b/pretty.c index 997f5837d5..4bf3be2dd9 100644 --- a/pretty.c +++ b/pretty.c @@ -30,8 +30,7 @@ enum cmit_fmt get_commit_format(const char *arg) if (*arg == '=') arg++; if (!prefixcmp(arg, "format:")) { - if (user_format) - free(user_format); + free(user_format); user_format = xstrdup(arg + 7); return CMIT_FMT_USERFORMAT; } diff --git a/remote.c b/remote.c index 6b56473f5b..ae1ef5708c 100644 --- a/remote.c +++ b/remote.c @@ -506,8 +506,7 @@ void free_refs(struct ref *ref) struct ref *next; while (ref) { next = ref->next; - if (ref->peer_ref) - free(ref->peer_ref); + free(ref->peer_ref); free(ref); ref = next; } diff --git a/setup.c b/setup.c index dc247a84c4..89c81e54e6 100644 --- a/setup.c +++ b/setup.c @@ -448,8 +448,7 @@ int check_repository_format_version(const char *var, const char *value) } else if (strcmp(var, "core.worktree") == 0) { if (!value) return config_error_nonbool(var); - if (git_work_tree_cfg) - free(git_work_tree_cfg); + free(git_work_tree_cfg); git_work_tree_cfg = xstrdup(value); inside_work_tree = -1; } diff --git a/sha1_name.c b/sha1_name.c index c2805e736b..9d088cc2ca 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -625,8 +625,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1) commit = pop_most_recent_commit(&list, ONELINE_SEEN); if (!parse_object(commit->object.sha1)) continue; - if (temp_commit_buffer) - free(temp_commit_buffer); + free(temp_commit_buffer); if (commit->buffer) p = commit->buffer; else { @@ -643,8 +642,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1) break; } } - if (temp_commit_buffer) - free(temp_commit_buffer); + free(temp_commit_buffer); free_commit_list(list); for (l = backup; l; l = l->next) clear_commit_marks(l->item, ONELINE_SEEN); diff --git a/xdiff-interface.c b/xdiff-interface.c index 4b8e5cca80..bba236428a 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -233,8 +233,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value) expression = value; if (regcomp(®->re, expression, 0)) die("Invalid regexp to look for hunk header: %s", expression); - if (buffer) - free(buffer); + free(buffer); value = ep + 1; } }