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 <phillip.wood123@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ævar Arnfjörð Bjarmason committed
Apr 13, 2022 at 22:01 UTC
bf20fe4ca8c8d2ba2087c9c44b4ee09a1d1467ed
5 files changed
+45
-30
builtin/rev-list.c
+2
-4
@@ -213,10 +213,8 @@ static void show_commit(struct commit *commit, void *data)
213
214
static void finish_commit(struct commit *commit)
215
{
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;
218
free_commit_buffer(the_repository->parsed_objects,
219
commit);
220
}
commit.c
+8
-11
@@ -397,17 +397,14 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b
397
398
if (item->object.parsed)
399
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;
408
409
tail += size;
410
if (tail <= bufptr + tree_entry_len + 1 || memcmp(bufptr, "tree ", 5) ||
contrib/coccinelle/free.cocci
+27
@@ -2,13 +2,21 @@
2
expression E;
3
@@
4
- if (E)
5
+(
6
free(E);
7
+|
8
+ free_commit_list(E);
9
+)
10
11
@@
12
expression E;
13
@@
14
- if (!E)
15
+(
16
free(E);
17
+|
18
+ free_commit_list(E);
19
+)
20
21
@@
22
expression E;
@@ -16,3 +24,22 @@ expression E;
24
- free(E);
25
+ FREE_AND_NULL(E);
26
- 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
+6
-11
@@ -1456,10 +1456,9 @@ static int limit_list(struct rev_info *revs)
1456
if (revs->left_only || revs->right_only)
1457
limit_left_right(newlist, revs);
1458
1459
- if (bottom) {
1459
+ if (bottom)
1460
limit_to_ancestry(bottom, newlist);
1461
- free_commit_list(bottom);
1462
- }
1461
+ free_commit_list(bottom);
1462
1463
/*
1464
* 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)
4079
* boundary commits anyway. (This is what the code has always
4080
* done.)
4081
*/
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;
4084
4085
/*
4086
* Put all of the actual boundary commits from revs->boundary_commits
@@ -4220,10 +4217,8 @@ struct commit *get_revision(struct rev_info *revs)
4217
graph_update(revs->graph, c);
4218
if (!c) {
4219
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;
4222
}
4223
return c;
4224
}
submodule.c
+2
-4
@@ -664,8 +664,7 @@ void show_submodule_diff_summary(struct diff_options *o, const char *path,
664
print_submodule_diff_summary(sub, &rev, o);
665
666
out:
667
- if (merge_bases)
668
- free_commit_list(merge_bases);
667
+ free_commit_list(merge_bases);
668
clear_commit_marks(left, ~0);
669
clear_commit_marks(right, ~0);
670
if (sub) {
@@ -754,8 +753,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
753
754
done:
755
strbuf_release(&sb);
757
- if (merge_bases)
758
- free_commit_list(merge_bases);
756
+ free_commit_list(merge_bases);
757
if (left)
758
clear_commit_marks(left, ~0);
759
if (right)