Skip to content

Commit 8e0f700

Browse files
Jim Meyeringgitster
authored andcommitted
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 <[email protected]>. Signed-off-by: Jim Meyering <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 22c430a commit 8e0f700

17 files changed

+28
-55
lines changed

builtin-blame.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,7 @@ static inline struct origin *origin_incref(struct origin *o)
123123
static void origin_decref(struct origin *o)
124124
{
125125
if (o && --o->refcnt <= 0) {
126-
if (o->file.ptr)
127-
free(o->file.ptr);
126+
free(o->file.ptr);
128127
free(o);
129128
}
130129
}

builtin-branch.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
126126
continue;
127127
}
128128

129-
if (name)
130-
free(name);
129+
free(name);
131130

132131
name = xstrdup(mkpath(fmt, argv[i]));
133132
if (!resolve_ref(name, sha1, 1, NULL)) {
@@ -172,8 +171,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
172171
}
173172
}
174173

175-
if (name)
176-
free(name);
174+
free(name);
177175

178176
return(ret);
179177
}
@@ -490,8 +488,7 @@ static void create_branch(const char *name, const char *start_name,
490488
if (write_ref_sha1(lock, sha1, msg) < 0)
491489
die("Failed to write ref: %s.", strerror(errno));
492490

493-
if (real_ref)
494-
free(real_ref);
491+
free(real_ref);
495492
}
496493

497494
static void rename_branch(const char *oldname, const char *newname, int force)

builtin-fast-export.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
196196
? strlen(reencoded) : message
197197
? strlen(message) : 0),
198198
reencoded ? reencoded : message ? message : "");
199-
if (reencoded)
200-
free(reencoded);
199+
free(reencoded);
201200

202201
for (i = 0, p = commit->parents; p; p = p->next) {
203202
int mark = get_object_mark(&p->item->object);

builtin-http-fetch.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,7 @@ int cmd_http_fetch(int argc, const char **argv, const char *prefix)
8080

8181
walker_free(walker);
8282

83-
if (rewritten_url)
84-
free(rewritten_url);
83+
free(rewritten_url);
8584

8685
return rc;
8786
}

builtin-pack-objects.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,8 +1428,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
14281428
* accounting lock. Compiler will optimize the strangeness
14291429
* away when THREADED_DELTA_SEARCH is not defined.
14301430
*/
1431-
if (trg_entry->delta_data)
1432-
free(trg_entry->delta_data);
1431+
free(trg_entry->delta_data);
14331432
cache_lock();
14341433
if (trg_entry->delta_data) {
14351434
delta_cache_size -= trg_entry->delta_size;

builtin-revert.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
397397
else
398398
return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
399399
}
400-
if (reencoded_message)
401-
free(reencoded_message);
400+
free(reencoded_message);
402401

403402
return 0;
404403
}

connect.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ struct ref **get_remote_heads(int in, struct ref **list,
6868

6969
name_len = strlen(name);
7070
if (len != name_len + 41) {
71-
if (server_capabilities)
72-
free(server_capabilities);
71+
free(server_capabilities);
7372
server_capabilities = xstrdup(name + name_len + 1);
7473
}
7574

diff.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v
118118
pp->next = funcname_pattern_list;
119119
funcname_pattern_list = pp;
120120
}
121-
if (pp->pattern)
122-
free(pp->pattern);
121+
free(pp->pattern);
123122
pp->pattern = xstrdup(value);
124123
return 0;
125124
}
@@ -492,10 +491,8 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
492491
ecbdata->diff_words->plus.text.size)
493492
diff_words_show(ecbdata->diff_words);
494493

495-
if (ecbdata->diff_words->minus.text.ptr)
496-
free (ecbdata->diff_words->minus.text.ptr);
497-
if (ecbdata->diff_words->plus.text.ptr)
498-
free (ecbdata->diff_words->plus.text.ptr);
494+
free (ecbdata->diff_words->minus.text.ptr);
495+
free (ecbdata->diff_words->plus.text.ptr);
499496
free(ecbdata->diff_words);
500497
ecbdata->diff_words = NULL;
501498
}

dir.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,7 @@ static struct path_simplify *create_simplify(const char **pathspec)
704704

705705
static void free_simplify(struct path_simplify *simplify)
706706
{
707-
if (simplify)
708-
free(simplify);
707+
free(simplify);
709708
}
710709

711710
int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec)

http-push.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -664,8 +664,7 @@ static void release_request(struct transfer_request *request)
664664
close(request->local_fileno);
665665
if (request->local_stream)
666666
fclose(request->local_stream);
667-
if (request->url != NULL)
668-
free(request->url);
667+
free(request->url);
669668
free(request);
670669
}
671670

@@ -1283,10 +1282,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
12831282
strbuf_release(&in_buffer);
12841283

12851284
if (lock->token == NULL || lock->timeout <= 0) {
1286-
if (lock->token != NULL)
1287-
free(lock->token);
1288-
if (lock->owner != NULL)
1289-
free(lock->owner);
1285+
free(lock->token);
1286+
free(lock->owner);
12901287
free(url);
12911288
free(lock);
12921289
lock = NULL;
@@ -1344,8 +1341,7 @@ static int unlock_remote(struct remote_lock *lock)
13441341
prev->next = prev->next->next;
13451342
}
13461343

1347-
if (lock->owner != NULL)
1348-
free(lock->owner);
1344+
free(lock->owner);
13491345
free(lock->url);
13501346
free(lock->token);
13511347
free(lock);
@@ -2035,8 +2031,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
20352031
}
20362032
free(url);
20372033

2038-
if (*symref != NULL)
2039-
free(*symref);
2034+
free(*symref);
20402035
*symref = NULL;
20412036
hashclr(sha1);
20422037

@@ -2435,8 +2430,7 @@ int main(int argc, char **argv)
24352430
}
24362431

24372432
cleanup:
2438-
if (rewritten_url)
2439-
free(rewritten_url);
2433+
free(rewritten_url);
24402434
if (info_ref_lock)
24412435
unlock_remote(info_ref_lock);
24422436
free(remote);

imap-send.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,
472472
if (socket_write( &imap->buf.sock, buf, bufl ) != bufl) {
473473
free( cmd->cmd );
474474
free( cmd );
475-
if (cb && cb->data)
475+
if (cb)
476476
free( cb->data );
477477
return NULL;
478478
}
@@ -858,8 +858,7 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
858858
normal:
859859
if (cmdp->cb.done)
860860
cmdp->cb.done( ctx, cmdp, resp );
861-
if (cmdp->cb.data)
862-
free( cmdp->cb.data );
861+
free( cmdp->cb.data );
863862
free( cmdp->cmd );
864863
free( cmdp );
865864
if (!tcmd || tcmd == cmdp)

interpolate.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ void interp_set_entry(struct interp *table, int slot, const char *value)
1111
char *oldval = table[slot].value;
1212
char *newval = NULL;
1313

14-
if (oldval)
15-
free(oldval);
14+
free(oldval);
1615

1716
if (value)
1817
newval = xstrdup(value);

pretty.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ enum cmit_fmt get_commit_format(const char *arg)
3030
if (*arg == '=')
3131
arg++;
3232
if (!prefixcmp(arg, "format:")) {
33-
if (user_format)
34-
free(user_format);
33+
free(user_format);
3534
user_format = xstrdup(arg + 7);
3635
return CMIT_FMT_USERFORMAT;
3736
}

remote.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,8 +506,7 @@ void free_refs(struct ref *ref)
506506
struct ref *next;
507507
while (ref) {
508508
next = ref->next;
509-
if (ref->peer_ref)
510-
free(ref->peer_ref);
509+
free(ref->peer_ref);
511510
free(ref);
512511
ref = next;
513512
}

setup.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,7 @@ int check_repository_format_version(const char *var, const char *value)
448448
} else if (strcmp(var, "core.worktree") == 0) {
449449
if (!value)
450450
return config_error_nonbool(var);
451-
if (git_work_tree_cfg)
452-
free(git_work_tree_cfg);
451+
free(git_work_tree_cfg);
453452
git_work_tree_cfg = xstrdup(value);
454453
inside_work_tree = -1;
455454
}

sha1_name.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -625,8 +625,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
625625
commit = pop_most_recent_commit(&list, ONELINE_SEEN);
626626
if (!parse_object(commit->object.sha1))
627627
continue;
628-
if (temp_commit_buffer)
629-
free(temp_commit_buffer);
628+
free(temp_commit_buffer);
630629
if (commit->buffer)
631630
p = commit->buffer;
632631
else {
@@ -643,8 +642,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
643642
break;
644643
}
645644
}
646-
if (temp_commit_buffer)
647-
free(temp_commit_buffer);
645+
free(temp_commit_buffer);
648646
free_commit_list(list);
649647
for (l = backup; l; l = l->next)
650648
clear_commit_marks(l->item, ONELINE_SEEN);

xdiff-interface.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)
233233
expression = value;
234234
if (regcomp(&reg->re, expression, 0))
235235
die("Invalid regexp to look for hunk header: %s", expression);
236-
if (buffer)
237-
free(buffer);
236+
free(buffer);
238237
value = ep + 1;
239238
}
240239
}

0 commit comments

Comments
 (0)