Skip to content

Commit bf20fe4

Browse files
avargitster
authored andcommitted
cocci: add and apply free_commit_list() rules
Add and apply coccinelle rules to remove "if (E)" before "free_commit_list(E)", the function can accept NULL, and further change cases where "E = NULL" followed to also be unconditionally. The code changes in this commit were entirely made by the coccinelle rule being added here, and applied with: make contrib/coccinelle/free.cocci.patch patch -p1 <contrib/coccinelle/free.cocci.patch The only manual intervention here is that the the relevant code in commit.c has been manually re-indented. Suggested-by: Phillip Wood <[email protected]> Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 89f45cf commit bf20fe4

File tree

5 files changed

+45
-30
lines changed

5 files changed

+45
-30
lines changed

builtin/rev-list.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,8 @@ static void show_commit(struct commit *commit, void *data)
213213

214214
static void finish_commit(struct commit *commit)
215215
{
216-
if (commit->parents) {
217-
free_commit_list(commit->parents);
218-
commit->parents = NULL;
219-
}
216+
free_commit_list(commit->parents);
217+
commit->parents = NULL;
220218
free_commit_buffer(the_repository->parsed_objects,
221219
commit);
222220
}

commit.c

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -397,17 +397,14 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b
397397

398398
if (item->object.parsed)
399399
return 0;
400-
401-
if (item->parents) {
402-
/*
403-
* Presumably this is leftover from an earlier failed parse;
404-
* clear it out in preparation for us re-parsing (we'll hit the
405-
* same error, but that's good, since it lets our caller know
406-
* the result cannot be trusted.
407-
*/
408-
free_commit_list(item->parents);
409-
item->parents = NULL;
410-
}
400+
/*
401+
* Presumably this is leftover from an earlier failed parse;
402+
* clear it out in preparation for us re-parsing (we'll hit the
403+
* same error, but that's good, since it lets our caller know
404+
* the result cannot be trusted.
405+
*/
406+
free_commit_list(item->parents);
407+
item->parents = NULL;
411408

412409
tail += size;
413410
if (tail <= bufptr + tree_entry_len + 1 || memcmp(bufptr, "tree ", 5) ||

contrib/coccinelle/free.cocci

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,44 @@
22
expression E;
33
@@
44
- if (E)
5+
(
56
free(E);
7+
|
8+
free_commit_list(E);
9+
)
610

711
@@
812
expression E;
913
@@
1014
- if (!E)
15+
(
1116
free(E);
17+
|
18+
free_commit_list(E);
19+
)
1220

1321
@@
1422
expression E;
1523
@@
1624
- free(E);
1725
+ FREE_AND_NULL(E);
1826
- E = NULL;
27+
28+
@@
29+
expression E;
30+
@@
31+
- if (E)
32+
- {
33+
free_commit_list(E);
34+
E = NULL;
35+
- }
36+
37+
@@
38+
expression E;
39+
statement S;
40+
@@
41+
- if (E) {
42+
+ if (E)
43+
S
44+
free_commit_list(E);
45+
- }

revision.c

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,10 +1456,9 @@ static int limit_list(struct rev_info *revs)
14561456
if (revs->left_only || revs->right_only)
14571457
limit_left_right(newlist, revs);
14581458

1459-
if (bottom) {
1459+
if (bottom)
14601460
limit_to_ancestry(bottom, newlist);
1461-
free_commit_list(bottom);
1462-
}
1461+
free_commit_list(bottom);
14631462

14641463
/*
14651464
* Check if any commits have become TREESAME by some of their parents
@@ -4080,10 +4079,8 @@ static void create_boundary_commit_list(struct rev_info *revs)
40804079
* boundary commits anyway. (This is what the code has always
40814080
* done.)
40824081
*/
4083-
if (revs->commits) {
4084-
free_commit_list(revs->commits);
4085-
revs->commits = NULL;
4086-
}
4082+
free_commit_list(revs->commits);
4083+
revs->commits = NULL;
40874084

40884085
/*
40894086
* Put all of the actual boundary commits from revs->boundary_commits
@@ -4220,10 +4217,8 @@ struct commit *get_revision(struct rev_info *revs)
42204217
graph_update(revs->graph, c);
42214218
if (!c) {
42224219
free_saved_parents(revs);
4223-
if (revs->previous_parents) {
4224-
free_commit_list(revs->previous_parents);
4225-
revs->previous_parents = NULL;
4226-
}
4220+
free_commit_list(revs->previous_parents);
4221+
revs->previous_parents = NULL;
42274222
}
42284223
return c;
42294224
}

submodule.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -664,8 +664,7 @@ void show_submodule_diff_summary(struct diff_options *o, const char *path,
664664
print_submodule_diff_summary(sub, &rev, o);
665665

666666
out:
667-
if (merge_bases)
668-
free_commit_list(merge_bases);
667+
free_commit_list(merge_bases);
669668
clear_commit_marks(left, ~0);
670669
clear_commit_marks(right, ~0);
671670
if (sub) {
@@ -754,8 +753,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
754753

755754
done:
756755
strbuf_release(&sb);
757-
if (merge_bases)
758-
free_commit_list(merge_bases);
756+
free_commit_list(merge_bases);
759757
if (left)
760758
clear_commit_marks(left, ~0);
761759
if (right)

0 commit comments

Comments
 (0)