# [PATCH v4 6/6] *.[ch] refactoring: make use of the FREE_AND_NULL() macro 18 messages from 2017-06-15 to 2017-06-28. Participants: Ævar Arnfjörð Bjarmason, Junio C Hamano, René Scharfe. Thread: https://gitlist.dev/t/46195 ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 6/6] *.[ch] refactoring: make use of the FREE_AND_NULL() macro Message-ID: <20170615231549.20085-7-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-7-avarab%40gmail.com In-Reply-To: <20170615231549.20085-1-avarab@gmail.com> ``` Replace occurrences of `free(ptr); ptr = NULL` which weren't caught by the coccinelle rule. These fall into two categories: - free/NULL assignments one after the other which coccinelle all put on one line, which is functionally equivalent code, but very ugly. - manually spotted occurrences where the NULL assignment isn't right after the free() call. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/am.c | 15 ++++----------- builtin/worktree.c | 6 ++---- commit-slab.h | 3 +-- credential.c | 6 ++---- gpg-interface.c | 15 +++++---------- grep.c | 9 +++------ help.c | 3 +-- line-log.c | 3 +-- prio-queue.c | 3 +-- refs/ref-cache.c | 3 +-- rerere.c | 3 +-- split-index.c | 3 +-- 12 files changed, 23 insertions(+), 49 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 138fb98537..80368b6fe6 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1072,17 +1072,10 @@ static void am_next(struct am_state *state) { struct object_id head; - free(state->author_name); - state->author_name = NULL; - - free(state->author_email); - state->author_email = NULL; - - free(state->author_date); - state->author_date = NULL; - - free(state->msg); - state->msg = NULL; + FREE_AND_NULL(state->author_name); + FREE_AND_NULL(state->author_email); + FREE_AND_NULL(state->author_date); + FREE_AND_NULL(state->msg); state->msg_len = 0; unlink(am_path(state, "author-script")); diff --git a/builtin/worktree.c b/builtin/worktree.c index 793306ea51..41d1c007a4 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -299,10 +299,8 @@ static int add_worktree(const char *path, const char *refname, } is_junk = 0; - free(junk_work_tree); - free(junk_git_dir); - junk_work_tree = NULL; - junk_git_dir = NULL; + FREE_AND_NULL(junk_work_tree); + FREE_AND_NULL(junk_git_dir); done: if (ret || !opts->keep_locked) { diff --git a/commit-slab.h b/commit-slab.h index 42d16dcded..333d81e370 100644 --- a/commit-slab.h +++ b/commit-slab.h @@ -82,8 +82,7 @@ static MAYBE_UNUSED void clear_ ##slabname(struct slabname *s) \ for (i = 0; i < s->slab_count; i++) \ free(s->slab[i]); \ s->slab_count = 0; \ - free(s->slab); \ - s->slab = NULL; \ + FREE_AND_NULL(s->slab); \ } \ \ static MAYBE_UNUSED elemtype *slabname## _at_peek(struct slabname *s, \ diff --git a/credential.c b/credential.c index 0ab247ff40..924ab58538 100644 --- a/credential.c +++ b/credential.c @@ -313,10 +313,8 @@ void credential_reject(struct credential *c) for (i = 0; i < c->helpers.nr; i++) credential_do(c, c->helpers.items[i].string, "erase"); - free(c->username); - c->username = NULL; - free(c->password); - c->password = NULL; + FREE_AND_NULL(c->username); + FREE_AND_NULL(c->password); c->approved = 0; } diff --git a/gpg-interface.c b/gpg-interface.c index e44cc27da1..3f377f89d0 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -13,16 +13,11 @@ static const char *gpg_program = "gpg"; void signature_check_clear(struct signature_check *sigc) { - free(sigc->payload); - free(sigc->gpg_output); - free(sigc->gpg_status); - free(sigc->signer); - free(sigc->key); - sigc->payload = NULL; - sigc->gpg_output = NULL; - sigc->gpg_status = NULL; - sigc->signer = NULL; - sigc->key = NULL; + FREE_AND_NULL(sigc->payload); + FREE_AND_NULL(sigc->gpg_output); + FREE_AND_NULL(sigc->gpg_status); + FREE_AND_NULL(sigc->signer); + FREE_AND_NULL(sigc->key); } static struct { diff --git a/grep.c b/grep.c index 11513ec06d..909f35a984 100644 --- a/grep.c +++ b/grep.c @@ -1763,12 +1763,9 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, void grep_source_clear(struct grep_source *gs) { - free(gs->name); - gs->name = NULL; - free(gs->path); - gs->path = NULL; - free(gs->identifier); - gs->identifier = NULL; + FREE_AND_NULL(gs->name); + FREE_AND_NULL(gs->path); + FREE_AND_NULL(gs->identifier); grep_source_clear_data(gs); } diff --git a/help.c b/help.c index db7f3d79a0..a2f48c0df3 100644 --- a/help.c +++ b/help.c @@ -267,9 +267,8 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) for (i = 0; i < old->cnt; i++) cmds->names[cmds->cnt++] = old->names[i]; - free(old->names); + FREE_AND_NULL(old->names); old->cnt = 0; - old->names = NULL; } /* An empirically derived magic number */ diff --git a/line-log.c b/line-log.c index 79eded7363..2d60c347ca 100644 --- a/line-log.c +++ b/line-log.c @@ -34,9 +34,8 @@ void range_set_init(struct range_set *rs, size_t prealloc) void range_set_release(struct range_set *rs) { - free(rs->ranges); + FREE_AND_NULL(rs->ranges); rs->alloc = rs->nr = 0; - rs->ranges = NULL; } /* dst must be uninitialized! */ diff --git a/prio-queue.c b/prio-queue.c index fc3860fdcb..126d096727 100644 --- a/prio-queue.c +++ b/prio-queue.c @@ -27,10 +27,9 @@ void prio_queue_reverse(struct prio_queue *queue) void clear_prio_queue(struct prio_queue *queue) { - free(queue->array); + FREE_AND_NULL(queue->array); queue->nr = 0; queue->alloc = 0; - queue->array = NULL; queue->insertion_ctr = 0; } diff --git a/refs/ref-cache.c b/refs/ref-cache.c index af2fcb2c12..76bb723c86 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -82,9 +82,8 @@ static void clear_ref_dir(struct ref_dir *dir) int i; for (i = 0; i < dir->nr; i++) free_ref_entry(dir->entries[i]); - free(dir->entries); + FREE_AND_NULL(dir->entries); dir->sorted = dir->nr = dir->alloc = 0; - dir->entries = NULL; } struct ref_entry *create_dir_entry(struct ref_cache *cache, diff --git a/rerere.c b/rerere.c index c26c29f87a..61f3804a1e 100644 --- a/rerere.c +++ b/rerere.c @@ -39,9 +39,8 @@ static void free_rerere_dirs(void) free(rerere_dir[i]->status); free(rerere_dir[i]); } - free(rerere_dir); + FREE_AND_NULL(rerere_dir); rerere_dir_nr = rerere_dir_alloc = 0; - rerere_dir = NULL; } static void free_rerere_id(struct string_list_item *item) diff --git a/split-index.c b/split-index.c index 49bd197f71..20477c6a48 100644 --- a/split-index.c +++ b/split-index.c @@ -174,10 +174,9 @@ void merge_base_index(struct index_state *istate) ewah_free(si->delete_bitmap); ewah_free(si->replace_bitmap); - free(si->saved_cache); + FREE_AND_NULL(si->saved_cache); si->delete_bitmap = NULL; si->replace_bitmap = NULL; - si->saved_cache = NULL; si->saved_cache_nr = 0; } -- 2.13.1.508.gb3defc5cc ``` ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 2/6] coccinelle: add a rule to make "type" code use FREE_AND_NULL() Message-ID: <20170615231549.20085-3-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-3-avarab%40gmail.com In-Reply-To: <20170615231549.20085-1-avarab@gmail.com> ``` Signed-off-by: Ævar Arnfjörð Bjarmason --- contrib/coccinelle/free.cocci | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci index c03ba737e5..35fb992621 100644 --- a/contrib/coccinelle/free.cocci +++ b/contrib/coccinelle/free.cocci @@ -9,3 +9,11 @@ expression E; @@ - if (!E) free(E); + +@@ +type T; +T *ptr; +@@ +- free(ptr); +- ptr = NULL; ++ FREE_AND_NULL(ptr); -- 2.13.1.508.gb3defc5cc ``` ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 0/6] Add a FREE_AND_NULL() wrapper macro Message-ID: <20170615231549.20085-1-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-1-avarab%40gmail.com In-Reply-To: ``` I didn't know about the coccinelle semantic patch facility. This adds coccinelle rules to do the code changes, and adds subsequent follow-up commits which apply that change, with the series ending with a patch that I manually authored. This fixes a bug in earlier versions of the series. I was mistakenly freeing the wrong thing in the tree-walk.c change, that change is now gone, and all the changes not made by coccinelle are in one commit, making it easier to review. Ævar Arnfjörð Bjarmason (6): git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL coccinelle: add a rule to make "type" code use FREE_AND_NULL() coccinelle: make use of the "type" FREE_AND_NULL() rule coccinelle: add a rule to make "expression" code use FREE_AND_NULL() coccinelle: make use of the "expression" FREE_AND_NULL() rule *.[ch] refactoring: make use of the FREE_AND_NULL() macro alias.c | 6 ++---- apply.c | 3 +-- attr.c | 6 ++---- blame.c | 3 +-- branch.c | 3 +-- builtin/am.c | 18 +++++------------- builtin/clean.c | 6 ++---- builtin/config.c | 6 ++---- builtin/index-pack.c | 6 ++---- builtin/pack-objects.c | 12 ++++-------- builtin/unpack-objects.c | 3 +-- builtin/worktree.c | 6 ++---- commit-slab.h | 3 +-- commit.c | 3 +-- config.c | 3 +-- contrib/coccinelle/free.cocci | 15 +++++++++++++++ credential.c | 9 +++------ diff-lib.c | 3 +-- diff.c | 6 ++---- diffcore-rename.c | 6 ++---- dir.c | 9 +++------ fast-import.c | 6 ++---- git-compat-util.h | 6 ++++++ gpg-interface.c | 15 +++++---------- grep.c | 12 ++++-------- help.c | 3 +-- http-push.c | 24 ++++++++---------------- http.c | 15 +++++---------- imap-send.c | 3 +-- line-log.c | 6 ++---- ll-merge.c | 3 +-- mailinfo.c | 3 +-- object.c | 3 +-- pathspec.c | 3 +-- prio-queue.c | 3 +-- read-cache.c | 6 ++---- ref-filter.c | 3 +-- refs/files-backend.c | 3 +-- refs/ref-cache.c | 3 +-- remote-testsvn.c | 3 +-- rerere.c | 3 +-- sequencer.c | 3 +-- sha1-array.c | 3 +-- sha1_file.c | 3 +-- split-index.c | 3 +-- transport-helper.c | 27 +++++++++------------------ transport.c | 3 +-- tree-diff.c | 6 ++---- tree.c | 3 +-- 49 files changed, 117 insertions(+), 195 deletions(-) -- 2.13.1.508.gb3defc5cc ``` ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 1/6] git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL Message-ID: <20170615231549.20085-2-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-2-avarab%40gmail.com In-Reply-To: <20170615231549.20085-1-avarab@gmail.com> ``` Add a FREE_AND_NULL() wrapper marco for the common pattern of freeing a pointer and assigning NULL to it right afterwards. The implementation is similar to the (currently unused) XDL_PTRFREE macro in xdiff/xmacros.h added in commit 3443546f6e ("Use a *real* built-in diff generator", 2006-03-24). The only difference is that free() is called unconditionally, see [1]. See [2] for a suggested alternative which does this via a function instead of a macro. As covered in replies to that message, while it's a viable approach, it would introduce caveats which this approach doesn't have, so that potential change is left to a future follow-up change. This merely allows us to translate exactly what we're doing now to a less verbose & idiomatic form using a macro, while guaranteeing that we don't introduce any functional changes. 1. (http://public-inbox.org/git/alpine.DEB.2.20.1608301948310.129229@virtualbox/) 2. <20170610032143.GA7880@starla> (https://public-inbox.org/git/20170610032143.GA7880@starla/) Signed-off-by: Ævar Arnfjörð Bjarmason --- git-compat-util.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 51ba4e6b3b..047172d173 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -808,6 +808,12 @@ extern char *xgetcwd(void); extern FILE *fopen_for_writing(const char *path); extern FILE *fopen_or_warn(const char *path, const char *mode); +/* + * FREE_AND_NULL(ptr) is like free(ptr) followed by ptr = NULL. Note + * that ptr is used twice, so don't pass e.g. ptr++. + */ +#define FREE_AND_NULL(p) do { free(p); (p) = NULL; } while (0) + #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc))) #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc))) -- 2.13.1.508.gb3defc5cc ``` ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: <20170615231549.20085-5-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-5-avarab%40gmail.com In-Reply-To: <20170615231549.20085-1-avarab@gmail.com> ``` A follow-up to the existing "type" rule added in an earlier change. This catches some occurrences that are missed by the previous rule. Signed-off-by: Ævar Arnfjörð Bjarmason --- contrib/coccinelle/free.cocci | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci index 35fb992621..f2d97e755b 100644 --- a/contrib/coccinelle/free.cocci +++ b/contrib/coccinelle/free.cocci @@ -17,3 +17,10 @@ T *ptr; - free(ptr); - ptr = NULL; + FREE_AND_NULL(ptr); + +@@ +expression E; +@@ +- free(E); +- E = NULL; ++ FREE_AND_NULL(E); -- 2.13.1.508.gb3defc5cc ``` ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 5/6] coccinelle: make use of the "expression" FREE_AND_NULL() rule Message-ID: <20170615231549.20085-6-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-6-avarab%40gmail.com In-Reply-To: <20170615231549.20085-1-avarab@gmail.com> ``` A follow-up to the existing "expression" rule added in an earlier change. This manually excludes a few occurrences, mostly things that resulted in many FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent change. Signed-off-by: Ævar Arnfjörð Bjarmason --- blame.c | 3 +-- ll-merge.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/blame.c b/blame.c index 843c845cba..1183943960 100644 --- a/blame.c +++ b/blame.c @@ -314,8 +314,7 @@ static void fill_origin_blob(struct diff_options *opt, static void drop_origin_blob(struct blame_origin *o) { if (o->file.ptr) { - free(o->file.ptr); - o->file.ptr = NULL; + FREE_AND_NULL(o->file.ptr); } } diff --git a/ll-merge.c b/ll-merge.c index ac0d4a5d78..91ff519cef 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -209,8 +209,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn, result->size = st.st_size; result->ptr = xmallocz(result->size); if (read_in_full(fd, result->ptr, result->size) != result->size) { - free(result->ptr); - result->ptr = NULL; + FREE_AND_NULL(result->ptr); result->size = 0; } close_bad: -- 2.13.1.508.gb3defc5cc ``` ## Ævar Arnfjörð Bjarmason, 2017-06-15 23:15 Subject: [PATCH v4 3/6] coccinelle: make use of the "type" FREE_AND_NULL() rule Message-ID: <20170615231549.20085-4-avarab@gmail.com> URL: https://gitlist.dev/e/20170615231549.20085-4-avarab%40gmail.com In-Reply-To: <20170615231549.20085-1-avarab@gmail.com> ``` Apply the result of the just-added coccinelle rule. This manually excludes a few occurrences, mostly things that resulted in many FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent change. Signed-off-by: Ævar Arnfjörð Bjarmason --- alias.c | 6 ++---- apply.c | 3 +-- attr.c | 6 ++---- branch.c | 3 +-- builtin/am.c | 3 +-- builtin/clean.c | 6 ++---- builtin/config.c | 6 ++---- builtin/index-pack.c | 6 ++---- builtin/pack-objects.c | 12 ++++-------- builtin/unpack-objects.c | 3 +-- commit.c | 3 +-- config.c | 3 +-- credential.c | 3 +-- diff-lib.c | 3 +-- diff.c | 6 ++---- diffcore-rename.c | 6 ++---- dir.c | 9 +++------ fast-import.c | 6 ++---- grep.c | 3 +-- http-push.c | 24 ++++++++---------------- http.c | 15 +++++---------- imap-send.c | 3 +-- line-log.c | 3 +-- mailinfo.c | 3 +-- object.c | 3 +-- pathspec.c | 3 +-- read-cache.c | 6 ++---- ref-filter.c | 3 +-- refs/files-backend.c | 3 +-- remote-testsvn.c | 3 +-- sequencer.c | 3 +-- sha1-array.c | 3 +-- sha1_file.c | 3 +-- transport-helper.c | 27 +++++++++------------------ transport.c | 3 +-- tree-diff.c | 6 ++---- tree.c | 3 +-- 37 files changed, 71 insertions(+), 142 deletions(-) diff --git a/alias.c b/alias.c index 3b90397a99..911481855f 100644 --- a/alias.c +++ b/alias.c @@ -47,8 +47,7 @@ int split_cmdline(char *cmdline, const char ***argv) src++; c = cmdline[src]; if (!c) { - free(*argv); - *argv = NULL; + FREE_AND_NULL(*argv); return -SPLIT_CMDLINE_BAD_ENDING; } } @@ -60,8 +59,7 @@ int split_cmdline(char *cmdline, const char ***argv) cmdline[dst] = 0; if (quoted) { - free(*argv); - *argv = NULL; + FREE_AND_NULL(*argv); return -SPLIT_CMDLINE_UNCLOSED_QUOTE; } diff --git a/apply.c b/apply.c index 854faa6779..e78de0affa 100644 --- a/apply.c +++ b/apply.c @@ -3705,8 +3705,7 @@ static int check_preimage(struct apply_state *state, is_new: patch->is_new = 1; patch->is_delete = 0; - free(patch->old_name); - patch->old_name = NULL; + FREE_AND_NULL(patch->old_name); return 0; } diff --git a/attr.c b/attr.c index 821203e2a9..ebdcfb0b8a 100644 --- a/attr.c +++ b/attr.c @@ -638,13 +638,11 @@ void attr_check_reset(struct attr_check *check) void attr_check_clear(struct attr_check *check) { - free(check->items); - check->items = NULL; + FREE_AND_NULL(check->items); check->alloc = 0; check->nr = 0; - free(check->all_attrs); - check->all_attrs = NULL; + FREE_AND_NULL(check->all_attrs); check->all_attrs_nr = 0; drop_attr_stack(&check->stack); diff --git a/branch.c b/branch.c index 985316eb76..2347cb8649 100644 --- a/branch.c +++ b/branch.c @@ -24,8 +24,7 @@ static int find_tracked_branch(struct remote *remote, void *priv) } else { free(tracking->spec.src); if (tracking->src) { - free(tracking->src); - tracking->src = NULL; + FREE_AND_NULL(tracking->src); } } tracking->spec.src = NULL; diff --git a/builtin/am.c b/builtin/am.c index 8881d73615..138fb98537 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -483,8 +483,7 @@ static int run_applypatch_msg_hook(struct am_state *state) ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); if (!ret) { - free(state->msg); - state->msg = NULL; + FREE_AND_NULL(state->msg); if (read_commit_msg(state) < 0) die(_("'%s' was deleted by the applypatch-msg hook"), am_path(state, "final-commit")); diff --git a/builtin/clean.c b/builtin/clean.c index 142bf668cf..bff5a07330 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -837,8 +837,7 @@ static void interactive_main_loop(void) int ret; ret = menus[*chosen].fn(); if (ret != MENU_RETURN_NO_LOOP) { - free(chosen); - chosen = NULL; + FREE_AND_NULL(chosen); if (!del_list.nr) { clean_print_color(CLEAN_COLOR_ERROR); printf_ln(_("No more files to clean, exiting.")); @@ -851,8 +850,7 @@ static void interactive_main_loop(void) quit_cmd(); } - free(chosen); - chosen = NULL; + FREE_AND_NULL(chosen); break; } } diff --git a/builtin/config.c b/builtin/config.c index 7f6c25d4d9..18c2158630 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -214,8 +214,7 @@ static int get_value(const char *key_, const char *regex_) key_regexp = (regex_t*)xmalloc(sizeof(regex_t)); if (regcomp(key_regexp, key, REG_EXTENDED)) { error("invalid key pattern: %s", key_); - free(key_regexp); - key_regexp = NULL; + FREE_AND_NULL(key_regexp); ret = CONFIG_INVALID_PATTERN; goto free_strings; } @@ -235,8 +234,7 @@ static int get_value(const char *key_, const char *regex_) regexp = (regex_t*)xmalloc(sizeof(regex_t)); if (regcomp(regexp, regex_, REG_EXTENDED)) { error("invalid pattern: %s", regex_); - free(regexp); - regexp = NULL; + FREE_AND_NULL(regexp); ret = CONFIG_INVALID_PATTERN; goto free_strings; } diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 04b9dcaf0f..f3b16520c2 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -388,8 +388,7 @@ static struct base_data *alloc_base_data(void) static void free_base_data(struct base_data *c) { if (c->data) { - free(c->data); - c->data = NULL; + FREE_AND_NULL(c->data); get_thread_data()->base_cache_used -= c->size; } } @@ -605,8 +604,7 @@ static void *unpack_data(struct object_entry *obj, git_inflate_end(&stream); free(inbuf); if (consume) { - free(data); - data = NULL; + FREE_AND_NULL(data); } return data; } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index f672225def..790615b2c3 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -264,8 +264,7 @@ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_ent * make sure no cached delta data remains from a * previous attempt before a pack split occurred. */ - free(entry->delta_data); - entry->delta_data = NULL; + FREE_AND_NULL(entry->delta_data); entry->z_delta_size = 0; } else if (entry->delta_data) { size = entry->delta_size; @@ -1375,12 +1374,10 @@ static void cleanup_preferred_base(void) if (!pbase_tree_cache[i]) continue; free(pbase_tree_cache[i]->tree_data); - free(pbase_tree_cache[i]); - pbase_tree_cache[i] = NULL; + FREE_AND_NULL(pbase_tree_cache[i]); } - free(done_pbase_paths); - done_pbase_paths = NULL; + FREE_AND_NULL(done_pbase_paths); done_pbase_paths_num = done_pbase_paths_alloc = 0; } @@ -1970,8 +1967,7 @@ static unsigned long free_unpacked(struct unpacked *n) n->index = NULL; if (n->data) { freed_mem += n->entry->size; - free(n->data); - n->data = NULL; + FREE_AND_NULL(n->data); } n->entry = NULL; n->depth = 0; diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 8bc9997767..60bfe48789 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -112,8 +112,7 @@ static void *get_data(unsigned long size) break; if (ret != Z_OK) { error("inflate returned %d", ret); - free(buf); - buf = NULL; + FREE_AND_NULL(buf); if (!recover) exit(1); has_errors = 1; diff --git a/commit.c b/commit.c index 99846d9bf4..cbfd689939 100644 --- a/commit.c +++ b/commit.c @@ -287,8 +287,7 @@ void free_commit_buffer(struct commit *commit) { struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit); if (v) { - free(v->buffer); - v->buffer = NULL; + FREE_AND_NULL(v->buffer); v->size = 0; } } diff --git a/config.c b/config.c index 34a139c40b..0501a60dd2 100644 --- a/config.c +++ b/config.c @@ -395,8 +395,7 @@ static int git_config_parse_key_1(const char *key, char **store_key, int *basele out_free_ret_1: if (store_key) { - free(*store_key); - *store_key = NULL; + FREE_AND_NULL(*store_key); } return -CONFIG_INVALID_KEY; } diff --git a/credential.c b/credential.c index aa996669fc..0ab247ff40 100644 --- a/credential.c +++ b/credential.c @@ -93,8 +93,7 @@ static void credential_apply_config(struct credential *c) c->configured = 1; if (!c->use_http_path && proto_is_http(c->protocol)) { - free(c->path); - c->path = NULL; + FREE_AND_NULL(c->path); } } diff --git a/diff-lib.c b/diff-lib.c index 76c8f185cd..7528f4163f 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -179,8 +179,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) free(dpath); continue; } - free(dpath); - dpath = NULL; + FREE_AND_NULL(dpath); /* * Show the diff for the 'ce' if we found the one diff --git a/diff.c b/diff.c index e35cf6c704..3561adf49f 100644 --- a/diff.c +++ b/diff.c @@ -1218,8 +1218,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata) regfree(ecbdata->diff_words->word_regex); free(ecbdata->diff_words->word_regex); } - free(ecbdata->diff_words); - ecbdata->diff_words = NULL; + FREE_AND_NULL(ecbdata->diff_words); } } @@ -2951,8 +2950,7 @@ void diff_free_filespec_blob(struct diff_filespec *s) void diff_free_filespec_data(struct diff_filespec *s) { diff_free_filespec_blob(s); - free(s->cnt_data); - s->cnt_data = NULL; + FREE_AND_NULL(s->cnt_data); } static void prep_temp_blob(const char *path, struct diff_tempfile *temp, diff --git a/diffcore-rename.c b/diffcore-rename.c index f7444c86bd..6d8daf9c00 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -667,11 +667,9 @@ void diffcore_rename(struct diff_options *options) for (i = 0; i < rename_dst_nr; i++) free_filespec(rename_dst[i].two); - free(rename_dst); - rename_dst = NULL; + FREE_AND_NULL(rename_dst); rename_dst_nr = rename_dst_alloc = 0; - free(rename_src); - rename_src = NULL; + FREE_AND_NULL(rename_src); rename_src_nr = rename_src_alloc = 0; return; } diff --git a/dir.c b/dir.c index 1759063817..7dc168b806 100644 --- a/dir.c +++ b/dir.c @@ -2117,8 +2117,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate, for (i = j = 0; j < dir->nr; j++) { if (i && check_dir_entry_contains(dir->entries[i - 1], dir->entries[j])) { - free(dir->entries[j]); - dir->entries[j] = NULL; + FREE_AND_NULL(dir->entries[j]); } else { dir->entries[i++] = dir->entries[j]; } @@ -2144,8 +2143,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate, dir->untracked->dir_invalidated)) istate->cache_changed |= UNTRACKED_CHANGED; if (dir->untracked != istate->untracked) { - free(dir->untracked); - dir->untracked = NULL; + FREE_AND_NULL(dir->untracked); } } return dir->nr; @@ -2488,8 +2486,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra strbuf_addbuf(out, &untracked->ident); strbuf_add(out, ouc, ouc_size(len)); - free(ouc); - ouc = NULL; + FREE_AND_NULL(ouc); if (!untracked->root) { varint_len = encode_varint(0, varbuf); diff --git a/fast-import.c b/fast-import.c index 9a22fc92c0..eeab927d5a 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1063,8 +1063,7 @@ static void end_packfile(void) close(pack_data->pack_fd); unlink_or_warn(pack_data->pack_name); } - free(pack_data); - pack_data = NULL; + FREE_AND_NULL(pack_data); running = 0; /* We can't carry a delta across packfiles. */ @@ -1149,8 +1148,7 @@ static int store_object( /* We cannot carry a delta into the new pack. */ if (delta) { - free(delta); - delta = NULL; + FREE_AND_NULL(delta); git_deflate_init(&s, pack_compression_level); s.next_in = (void *)dat->buf; diff --git a/grep.c b/grep.c index d03d424e5c..11513ec06d 100644 --- a/grep.c +++ b/grep.c @@ -1778,8 +1778,7 @@ void grep_source_clear_data(struct grep_source *gs) case GREP_SOURCE_FILE: case GREP_SOURCE_SHA1: case GREP_SOURCE_SUBMODULE: - free(gs->buf); - gs->buf = NULL; + FREE_AND_NULL(gs->buf); gs->size = 0; break; case GREP_SOURCE_BUF: diff --git a/http-push.c b/http-push.c index 67c4d4b472..c91f40a610 100644 --- a/http-push.c +++ b/http-push.c @@ -291,8 +291,7 @@ static void start_mkcol(struct transfer_request *request) request->state = RUN_MKCOL; } else { request->state = ABORTED; - free(request->url); - request->url = NULL; + FREE_AND_NULL(request->url); } } #endif @@ -409,8 +408,7 @@ static void start_put(struct transfer_request *request) request->state = RUN_PUT; } else { request->state = ABORTED; - free(request->url); - request->url = NULL; + FREE_AND_NULL(request->url); } } @@ -432,8 +430,7 @@ static void start_move(struct transfer_request *request) request->state = RUN_MOVE; } else { request->state = ABORTED; - free(request->url); - request->url = NULL; + FREE_AND_NULL(request->url); } } @@ -526,8 +523,7 @@ static void finish_request(struct transfer_request *request) /* URL is reused for MOVE after PUT */ if (request->state != RUN_PUT) { - free(request->url); - request->url = NULL; + FREE_AND_NULL(request->url); } if (request->state == RUN_MKCOL) { @@ -803,8 +799,7 @@ xml_start_tag(void *userData, const char *name, const char **atts) } xsnprintf(ctx->name + old_namelen, ctx->len - old_namelen, ".%s", c); - free(ctx->cdata); - ctx->cdata = NULL; + FREE_AND_NULL(ctx->cdata); ctx->userFunc(ctx, 0); } @@ -932,8 +927,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) free(lock->token); free(lock->owner); free(url); - free(lock); - lock = NULL; + FREE_AND_NULL(lock); } else { lock->url = url; lock->start_time = time(NULL); @@ -1105,8 +1099,7 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed) ls->dentry_flags |= IS_DIR; } } else if (!strcmp(ctx->name, DAV_PROPFIND_RESP)) { - free(ls->dentry_name); - ls->dentry_name = NULL; + FREE_AND_NULL(ls->dentry_name); ls->dentry_flags = 0; } } @@ -1547,8 +1540,7 @@ static void fetch_symref(const char *path, char **symref, struct object_id *oid) curl_errorstr); free(url); - free(*symref); - *symref = NULL; + FREE_AND_NULL(*symref); oidclr(oid); if (buffer.len == 0) diff --git a/http.c b/http.c index d2e11ec6f0..77f46bef0f 100644 --- a/http.c +++ b/http.c @@ -1026,8 +1026,7 @@ void http_cleanup(void) if (proxy_auth.password) { memset(proxy_auth.password, 0, strlen(proxy_auth.password)); - free(proxy_auth.password); - proxy_auth.password = NULL; + FREE_AND_NULL(proxy_auth.password); } free((void *)curl_proxyuserpwd); @@ -1038,13 +1037,11 @@ void http_cleanup(void) if (cert_auth.password != NULL) { memset(cert_auth.password, 0, strlen(cert_auth.password)); - free(cert_auth.password); - cert_auth.password = NULL; + FREE_AND_NULL(cert_auth.password); } ssl_cert_password_required = 0; - free(cached_accept_language); - cached_accept_language = NULL; + FREE_AND_NULL(cached_accept_language); } struct active_request_slot *get_active_slot(void) @@ -1896,8 +1893,7 @@ static char *fetch_pack_index(unsigned char *sha1, const char *base_url) if (http_get_file(url, tmp, NULL) != HTTP_OK) { error("Unable to get pack index %s", url); - free(tmp); - tmp = NULL; + FREE_AND_NULL(tmp); } free(url); @@ -2328,8 +2324,7 @@ void release_http_object_request(struct http_object_request *freq) freq->localfile = -1; } if (freq->url != NULL) { - free(freq->url); - freq->url = NULL; + FREE_AND_NULL(freq->url); } if (freq->slot != NULL) { freq->slot->callback_func = NULL; diff --git a/imap-send.c b/imap-send.c index 857591660f..40ddb496e6 100644 --- a/imap-send.c +++ b/imap-send.c @@ -776,8 +776,7 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd) offsetof(struct imap_cmd, next)); if (cmdp->cb.data) { n = socket_write(&imap->buf.sock, cmdp->cb.data, cmdp->cb.dlen); - free(cmdp->cb.data); - cmdp->cb.data = NULL; + FREE_AND_NULL(cmdp->cb.data); if (n != (int)cmdp->cb.dlen) return RESP_BAD; } else if (cmdp->cb.cont) { diff --git a/line-log.c b/line-log.c index b9087814b8..79eded7363 100644 --- a/line-log.c +++ b/line-log.c @@ -610,8 +610,7 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) line_log_data_insert(&ranges, full_name, begin, end); free_filespec(spec); - free(ends); - ends = NULL; + FREE_AND_NULL(ends); } for (p = ranges; p; p = p->next) diff --git a/mailinfo.c b/mailinfo.c index f92cb9f729..44f829edb1 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -919,8 +919,7 @@ static int handle_boundary(struct mailinfo *mi, struct strbuf *line) /* we hit an end boundary */ /* pop the current boundary off the stack */ strbuf_release(*(mi->content_top)); - free(*(mi->content_top)); - *(mi->content_top) = NULL; + FREE_AND_NULL(*(mi->content_top)); /* technically won't happen as is_multipart_boundary() will fail first. But just in case.. diff --git a/object.c b/object.c index 06ba3a11d8..f818777412 100644 --- a/object.c +++ b/object.c @@ -377,8 +377,7 @@ void object_array_clear(struct object_array *array) int i; for (i = 0; i < array->nr; i++) object_array_release_entry(&array->objects[i]); - free(array->objects); - array->objects = NULL; + FREE_AND_NULL(array->objects); array->nr = array->alloc = 0; } diff --git a/pathspec.c b/pathspec.c index 828405021f..54aab8bc01 100644 --- a/pathspec.c +++ b/pathspec.c @@ -662,7 +662,6 @@ void clear_pathspec(struct pathspec *pathspec) attr_check_free(pathspec->items[i].attr_check); } - free(pathspec->items); - pathspec->items = NULL; + FREE_AND_NULL(pathspec->items); pathspec->nr = 0; } diff --git a/read-cache.c b/read-cache.c index bc156a133e..66c413657e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1888,8 +1888,7 @@ int discard_index(struct index_state *istate) free_name_hash(istate); cache_tree_free(&(istate->cache_tree)); istate->initialized = 0; - free(istate->cache); - istate->cache = NULL; + FREE_AND_NULL(istate->cache); istate->cache_alloc = 0; discard_split_index(istate); free_untracked_cache(istate->untracked); @@ -2603,8 +2602,7 @@ void *read_blob_data_from_index(const struct index_state *istate, void stat_validity_clear(struct stat_validity *sv) { - free(sv->sd); - sv->sd = NULL; + FREE_AND_NULL(sv->sd); } int stat_validity_check(struct stat_validity *sv, const char *path) diff --git a/ref-filter.c b/ref-filter.c index ab32bc9c31..72e6cb8ecc 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1891,8 +1891,7 @@ void ref_array_clear(struct ref_array *array) for (i = 0; i < array->nr; i++) free_array_item(array->items[i]); - free(array->items); - array->items = NULL; + FREE_AND_NULL(array->items); array->nr = array->alloc = 0; } diff --git a/refs/files-backend.c b/refs/files-backend.c index d8b3f73147..c1bd99d60f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2944,8 +2944,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, head_oid.hash, &head_type); if (head_ref && !(head_type & REF_ISSYMREF)) { - free(head_ref); - head_ref = NULL; + FREE_AND_NULL(head_ref); } /* diff --git a/remote-testsvn.c b/remote-testsvn.c index 50404ef343..078b0c3139 100644 --- a/remote-testsvn.c +++ b/remote-testsvn.c @@ -66,8 +66,7 @@ static char *read_ref_note(const unsigned char sha1[20]) else if (!msglen || type != OBJ_BLOB) { error("Note contains unusable content. " "Is something else using this notes tree? %s", notes_ref); - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } free_notes(NULL); return msg; diff --git a/sequencer.c b/sequencer.c index 5282fb849c..070b42f240 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1211,8 +1211,7 @@ struct todo_list { static void todo_list_release(struct todo_list *todo_list) { strbuf_release(&todo_list->buf); - free(todo_list->items); - todo_list->items = NULL; + FREE_AND_NULL(todo_list->items); todo_list->nr = todo_list->alloc = 0; } diff --git a/sha1-array.c b/sha1-array.c index 7d646ab5b8..838b3bf847 100644 --- a/sha1-array.c +++ b/sha1-array.c @@ -35,8 +35,7 @@ int oid_array_lookup(struct oid_array *array, const struct object_id *oid) void oid_array_clear(struct oid_array *array) { - free(array->oid); - array->oid = NULL; + FREE_AND_NULL(array->oid); array->nr = 0; array->alloc = 0; array->sorted = 0; diff --git a/sha1_file.c b/sha1_file.c index 59a4ed2ed3..d67c88bb8d 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -610,8 +610,7 @@ char *compute_alternate_path(const char *path, struct strbuf *err) out: if (seen_error) { - free(ref_git); - ref_git = NULL; + FREE_AND_NULL(ref_git); } return ref_git; diff --git a/transport-helper.c b/transport-helper.c index 36408046eb..33cff38cc0 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -242,8 +242,7 @@ static int disconnect_helper(struct transport *transport) close(data->helper->out); fclose(data->out); res = finish_command(data->helper); - free(data->helper); - data->helper = NULL; + FREE_AND_NULL(data->helper); } return res; } @@ -711,43 +710,35 @@ static int push_update_ref_status(struct strbuf *buf, if (!strcmp(msg, "no match")) { status = REF_STATUS_NONE; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "up to date")) { status = REF_STATUS_UPTODATE; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "non-fast forward")) { status = REF_STATUS_REJECT_NONFASTFORWARD; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "already exists")) { status = REF_STATUS_REJECT_ALREADY_EXISTS; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "fetch first")) { status = REF_STATUS_REJECT_FETCH_FIRST; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "needs force")) { status = REF_STATUS_REJECT_NEEDS_FORCE; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "stale info")) { status = REF_STATUS_REJECT_STALE; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } else if (!strcmp(msg, "forced update")) { forced = 1; - free(msg); - msg = NULL; + FREE_AND_NULL(msg); } } diff --git a/transport.c b/transport.c index 9bfcf870f9..cf35088001 100644 --- a/transport.c +++ b/transport.c @@ -1145,8 +1145,7 @@ void transport_unlock_pack(struct transport *transport) { if (transport->pack_lockfile) { unlink_or_warn(transport->pack_lockfile); - free(transport->pack_lockfile); - transport->pack_lockfile = NULL; + FREE_AND_NULL(transport->pack_lockfile); } } diff --git a/tree-diff.c b/tree-diff.c index e164e532b2..1e1b393d08 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -140,8 +140,7 @@ static struct combine_diff_path *path_appendnew(struct combine_diff_path *last, /* if last->next is !NULL - it is a pre-allocated memory, we can reuse */ p = last->next; if (p && (alloclen > (intptr_t)p->next)) { - free(p); - p = NULL; + FREE_AND_NULL(p); } if (!p) { @@ -559,8 +558,7 @@ struct combine_diff_path *diff_tree_paths( * (see path_appendnew() for details about why) */ if (p->next) { - free(p->next); - p->next = NULL; + FREE_AND_NULL(p->next); } return p; diff --git a/tree.c b/tree.c index 603b29ee80..614f91586d 100644 --- a/tree.c +++ b/tree.c @@ -226,8 +226,7 @@ int parse_tree_gently(struct tree *item, int quiet_on_missing) void free_tree_buffer(struct tree *tree) { - free(tree->buffer); - tree->buffer = NULL; + FREE_AND_NULL(tree->buffer); tree->size = 0; tree->object.parsed = 0; } -- 2.13.1.508.gb3defc5cc ``` ## Junio C Hamano, 2017-06-16 19:43 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: URL: https://gitlist.dev/e/xmqq7f0byayu.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20170615231549.20085-5-avarab@gmail.com> ``` Ævar Arnfjörð Bjarmason writes: > A follow-up to the existing "type" rule added in an earlier > change. This catches some occurrences that are missed by the previous > rule. > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- Hmph, I wonder if the "type" thing is really needed. Over there, "ptr" is an expression and we can find "free(ptr); ptr = NULL" with the rule in this patch already, no? In any case, I'll queue these patches as-is. Thanks for working on this. > contrib/coccinelle/free.cocci | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci > index 35fb992621..f2d97e755b 100644 > --- a/contrib/coccinelle/free.cocci > +++ b/contrib/coccinelle/free.cocci > @@ -17,3 +17,10 @@ T *ptr; > - free(ptr); > - ptr = NULL; > + FREE_AND_NULL(ptr); > + > +@@ > +expression E; > +@@ > +- free(E); > +- E = NULL; > ++ FREE_AND_NULL(E); ``` ## René Scharfe, 2017-06-25 08:01 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: <2122b01f-7627-cd1b-c7df-751c076f90ff@web.de> URL: https://gitlist.dev/e/2122b01f-7627-cd1b-c7df-751c076f90ff%40web.de In-Reply-To: ``` Am 16.06.2017 um 21:43 schrieb Junio C Hamano: > Ævar Arnfjörð Bjarmason writes: > >> A follow-up to the existing "type" rule added in an earlier >> change. This catches some occurrences that are missed by the previous >> rule. >> >> Signed-off-by: Ævar Arnfjörð Bjarmason >> --- > > Hmph, I wonder if the "type" thing is really needed. Over there, > "ptr" is an expression and we can find "free(ptr); ptr = NULL" with > the rule in this patch already, no? Indeed. How about this on top of master? -- >8 -- Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules There are two rules for using FREE_AND_NULL in free.cocci, one for pointer types and one for expressions. Both cause coccinelle to remove empty lines and even newline characters between replacements for some reason; consecutive "free(x);/x=NULL;" sequences end up as multiple FREE_AND_NULL calls on the same time. Remove the type rule, as the expression rule already covers it, and rearrange the lines of the latter to place the addition of FREE_AND_NULL between the removals, which causes coccinelle to leave surrounding whitespace untouched. Signed-off-by: Rene Scharfe --- contrib/coccinelle/free.cocci | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci index f2d97e755b..4490069df9 100644 --- a/contrib/coccinelle/free.cocci +++ b/contrib/coccinelle/free.cocci @@ -11,16 +11,8 @@ expression E; free(E); @@ -type T; -T *ptr; -@@ -- free(ptr); -- ptr = NULL; -+ FREE_AND_NULL(ptr); - -@@ expression E; @@ - free(E); -- E = NULL; + FREE_AND_NULL(E); +- E = NULL; -- 2.13.2 ``` ## Ævar Arnfjörð Bjarmason, 2017-06-28 21:39 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: <87injfkd0j.fsf@gmail.com> URL: https://gitlist.dev/e/87injfkd0j.fsf%40gmail.com In-Reply-To: <2122b01f-7627-cd1b-c7df-751c076f90ff@web.de> ``` On Sun, Jun 25 2017, René Scharfe jotted: > Am 16.06.2017 um 21:43 schrieb Junio C Hamano: >> Ævar Arnfjörð Bjarmason writes: >> >>> A follow-up to the existing "type" rule added in an earlier >>> change. This catches some occurrences that are missed by the previous >>> rule. >>> >>> Signed-off-by: Ævar Arnfjörð Bjarmason >>> --- >> >> Hmph, I wonder if the "type" thing is really needed. Over there, >> "ptr" is an expression and we can find "free(ptr); ptr = NULL" with >> the rule in this patch already, no? > > Indeed. How about this on top of master? > > -- >8 -- > Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules > > There are two rules for using FREE_AND_NULL in free.cocci, one for > pointer types and one for expressions. Both cause coccinelle to remove > empty lines and even newline characters between replacements for some > reason; consecutive "free(x);/x=NULL;" sequences end up as multiple > FREE_AND_NULL calls on the same time. > > Remove the type rule, as the expression rule already covers it, and > rearrange the lines of the latter to place the addition of FREE_AND_NULL > between the removals, which causes coccinelle to leave surrounding > whitespace untouched. > > Signed-off-by: Rene Scharfe > --- > contrib/coccinelle/free.cocci | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci > index f2d97e755b..4490069df9 100644 > --- a/contrib/coccinelle/free.cocci > +++ b/contrib/coccinelle/free.cocci > @@ -11,16 +11,8 @@ expression E; > free(E); > > @@ > -type T; > -T *ptr; > -@@ > -- free(ptr); > -- ptr = NULL; > -+ FREE_AND_NULL(ptr); > - > -@@ > expression E; > @@ > - free(E); > -- E = NULL; > + FREE_AND_NULL(E); > +- E = NULL; Late reply, sorry. What version of coccinelle are you running? I have 1.0.4 (from Debian) and can't get this to produce the same results as what I have. On top of next, I did: Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" And then generated the patch as usual with `make coccicheck`, and applied it. This has your change. I then re-applied the manual "*.[ch] refactoring" changes This results in this diff with next: $ git diff --stat origin/next.. -- '*.[ch]' builtin/am.c | 3 ++- builtin/clean.c | 6 ++++-- builtin/config.c | 6 ++++-- builtin/index-pack.c | 6 ++++-- builtin/pack-objects.c | 12 ++++++++---- builtin/unpack-objects.c | 3 ++- fast-import.c | 6 ++++-- http-push.c | 24 ++++++++++++++++-------- http.c | 15 ++++++++++----- imap-send.c | 3 ++- ref-filter.c | 3 ++- refs/files-backend.c | 3 ++- remote-testsvn.c | 3 ++- sequencer.c | 3 ++- sha1-array.c | 3 ++- sha1_file.c | 3 ++- transport-helper.c | 27 ++++++++++++++++++--------- transport.c | 3 ++- tree-diff.c | 6 ++++-- tree.c | 3 ++- 20 files changed, 94 insertions(+), 47 deletions(-) These are all cases where we now miss things that should use FREE_AND_NULL(), e.g.: diff --git a/builtin/am.c b/builtin/am.c index c973bd96dc..2f89338ed7 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state) ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); if (!ret) { - FREE_AND_NULL(state->msg); + free(state->msg); + state->msg = NULL; if (read_commit_msg(state) < 0) die(_("'%s' was deleted by the applypatch-msg hook"), am_path(state, "final-commit")); So it looks to me like removing the "type T" rule breaks a lot of things, but that the change you made to the expression rule is good, and we should do that for the "type" rule as well. Your commit says the "expression rule already covers it", but this doesn't seem to be the case at all. As an aside: Junio, did you mean to apply f8bb4631fb to next this way? Looks like a mis-applied scissor commit. ``` ## Junio C Hamano, 2017-06-28 22:00 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: URL: https://gitlist.dev/e/xmqqlgobvklj.fsf%40gitster.mtv.corp.google.com In-Reply-To: <87injfkd0j.fsf@gmail.com> ``` Ævar Arnfjörð Bjarmason writes: > Late reply, sorry. What version of coccinelle are you running? I have > 1.0.4 (from Debian) and can't get this to produce the same results as > what I have. > > On top of next, I did: > > Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" > Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" > Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" > > And then generated the patch as usual with `make coccicheck`, and > applied it. This has your change. > > I then re-applied the manual "*.[ch] refactoring" changes > > This results in this diff with next: > ... > > These are all cases where we now miss things that should use > FREE_AND_NULL(), e.g.: > > diff --git a/builtin/am.c b/builtin/am.c > index c973bd96dc..2f89338ed7 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state) > ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); > > if (!ret) { > - FREE_AND_NULL(state->msg); > + free(state->msg); > + state->msg = NULL; > if (read_commit_msg(state) < 0) > die(_("'%s' was deleted by the applypatch-msg hook"), > am_path(state, "final-commit")); > > So it looks to me like removing the "type T" rule breaks a lot of > things, but that the change you made to the expression rule is good, and > we should do that for the "type" rule as well. Your commit says the > "expression rule already covers it", but this doesn't seem to be the > case at all. OK, if an older version of the tool that is otherwise still usable needs two rules, let's keep the "type T" one by reverting the change out of 'next' and then have a patch that only rearranges the lines, something like the attached. -- >8 -- From: René Scharfe Date: Sun, 25 Jun 2017 10:01:04 +0200 Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules There are two rules for using FREE_AND_NULL in free.cocci, one for pointer types and one for expressions. Both cause coccinelle to remove empty lines and even newline characters between replacements for some reason; consecutive "free(x);/x=NULL;" sequences end up as multiple FREE_AND_NULL calls on the same time. Rearrange the lines to place the addition of FREE_AND_NULL between the removals, which causes coccinelle to leave surrounding whitespace untouched. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- contrib/coccinelle/free.cocci | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci index f2d97e755b..179c185d85 100644 --- a/contrib/coccinelle/free.cocci +++ b/contrib/coccinelle/free.cocci @@ -15,12 +15,12 @@ type T; T *ptr; @@ - free(ptr); -- ptr = NULL; + FREE_AND_NULL(ptr); +- ptr = NULL; @@ expression E; @@ - free(E); -- E = NULL; + FREE_AND_NULL(E); +- E = NULL; -- 2.13.2-659-g904a3dfd54 ``` ## Ævar Arnfjörð Bjarmason, 2017-06-28 22:17 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: <87fuejkb9m.fsf@gmail.com> URL: https://gitlist.dev/e/87fuejkb9m.fsf%40gmail.com In-Reply-To: ``` On Wed, Jun 28 2017, Junio C. Hamano jotted: > Ævar Arnfjörð Bjarmason writes: > >> Late reply, sorry. What version of coccinelle are you running? I have >> 1.0.4 (from Debian) and can't get this to produce the same results as >> what I have. >> >> On top of next, I did: >> >> Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" >> Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" >> Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" >> >> And then generated the patch as usual with `make coccicheck`, and >> applied it. This has your change. >> >> I then re-applied the manual "*.[ch] refactoring" changes >> >> This results in this diff with next: >> ... >> >> These are all cases where we now miss things that should use >> FREE_AND_NULL(), e.g.: >> >> diff --git a/builtin/am.c b/builtin/am.c >> index c973bd96dc..2f89338ed7 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state) >> ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); >> >> if (!ret) { >> - FREE_AND_NULL(state->msg); >> + free(state->msg); >> + state->msg = NULL; >> if (read_commit_msg(state) < 0) >> die(_("'%s' was deleted by the applypatch-msg hook"), >> am_path(state, "final-commit")); >> >> So it looks to me like removing the "type T" rule breaks a lot of >> things, but that the change you made to the expression rule is good, and >> we should do that for the "type" rule as well. Your commit says the >> "expression rule already covers it", but this doesn't seem to be the >> case at all. > > OK, if an older version of the tool that is otherwise still usable > needs two rules, let's keep the "type T" one by reverting the change > out of 'next' and then have a patch that only rearranges the lines, > something like the attached. > > -- >8 -- > From: René Scharfe > Date: Sun, 25 Jun 2017 10:01:04 +0200 > Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules > > There are two rules for using FREE_AND_NULL in free.cocci, one for > pointer types and one for expressions. Both cause coccinelle to remove > empty lines and even newline characters between replacements for some > reason; consecutive "free(x);/x=NULL;" sequences end up as multiple > FREE_AND_NULL calls on the same time. > > Rearrange the lines to place the addition of FREE_AND_NULL between > the removals, which causes coccinelle to leave surrounding > whitespace untouched. > > Signed-off-by: Rene Scharfe > Signed-off-by: Junio C Hamano > --- > contrib/coccinelle/free.cocci | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci > index f2d97e755b..179c185d85 100644 > --- a/contrib/coccinelle/free.cocci > +++ b/contrib/coccinelle/free.cocci > @@ -15,12 +15,12 @@ type T; > T *ptr; > @@ > - free(ptr); > -- ptr = NULL; > + FREE_AND_NULL(ptr); > +- ptr = NULL; > > @@ > expression E; > @@ > - free(E); > -- E = NULL; > + FREE_AND_NULL(E); > +- E = NULL; Looks good. ``` ## René Scharfe, 2017-06-28 22:21 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: <433f9fb7-a202-1091-46de-04535b2a49e1@web.de> URL: https://gitlist.dev/e/433f9fb7-a202-1091-46de-04535b2a49e1%40web.de In-Reply-To: <87injfkd0j.fsf@gmail.com> ``` Am 28.06.2017 um 23:39 schrieb Ævar Arnfjörð Bjarmason: > > On Sun, Jun 25 2017, René Scharfe jotted: > >> Am 16.06.2017 um 21:43 schrieb Junio C Hamano: >>> Ævar Arnfjörð Bjarmason writes: >>> >>>> A follow-up to the existing "type" rule added in an earlier >>>> change. This catches some occurrences that are missed by the previous >>>> rule. >>>> >>>> Signed-off-by: Ævar Arnfjörð Bjarmason >>>> --- >>> >>> Hmph, I wonder if the "type" thing is really needed. Over there, >>> "ptr" is an expression and we can find "free(ptr); ptr = NULL" with >>> the rule in this patch already, no? >> >> Indeed. How about this on top of master? >> >> -- >8 -- >> Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules >> >> There are two rules for using FREE_AND_NULL in free.cocci, one for >> pointer types and one for expressions. Both cause coccinelle to remove >> empty lines and even newline characters between replacements for some >> reason; consecutive "free(x);/x=NULL;" sequences end up as multiple >> FREE_AND_NULL calls on the same time. >> >> Remove the type rule, as the expression rule already covers it, and >> rearrange the lines of the latter to place the addition of FREE_AND_NULL >> between the removals, which causes coccinelle to leave surrounding >> whitespace untouched. >> >> Signed-off-by: Rene Scharfe >> --- >> contrib/coccinelle/free.cocci | 10 +--------- >> 1 file changed, 1 insertion(+), 9 deletions(-) >> >> diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci >> index f2d97e755b..4490069df9 100644 >> --- a/contrib/coccinelle/free.cocci >> +++ b/contrib/coccinelle/free.cocci >> @@ -11,16 +11,8 @@ expression E; >> free(E); >> >> @@ >> -type T; >> -T *ptr; >> -@@ >> -- free(ptr); >> -- ptr = NULL; >> -+ FREE_AND_NULL(ptr); >> - >> -@@ >> expression E; >> @@ >> - free(E); >> -- E = NULL; >> + FREE_AND_NULL(E); >> +- E = NULL; > > Late reply, sorry. What version of coccinelle are you running? I have > 1.0.4 (from Debian) and can't get this to produce the same results as > what I have. > > On top of next, I did: > > Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" > Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" > Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" > > And then generated the patch as usual with `make coccicheck`, and > applied it. This has your change. > > I then re-applied the manual "*.[ch] refactoring" changes > > This results in this diff with next: > > $ git diff --stat origin/next.. -- '*.[ch]' > builtin/am.c | 3 ++- > builtin/clean.c | 6 ++++-- > builtin/config.c | 6 ++++-- > builtin/index-pack.c | 6 ++++-- > builtin/pack-objects.c | 12 ++++++++---- > builtin/unpack-objects.c | 3 ++- > fast-import.c | 6 ++++-- > http-push.c | 24 ++++++++++++++++-------- > http.c | 15 ++++++++++----- > imap-send.c | 3 ++- > ref-filter.c | 3 ++- > refs/files-backend.c | 3 ++- > remote-testsvn.c | 3 ++- > sequencer.c | 3 ++- > sha1-array.c | 3 ++- > sha1_file.c | 3 ++- > transport-helper.c | 27 ++++++++++++++++++--------- > transport.c | 3 ++- > tree-diff.c | 6 ++++-- > tree.c | 3 ++- > 20 files changed, 94 insertions(+), 47 deletions(-) > > These are all cases where we now miss things that should use > FREE_AND_NULL(), e.g.: > > diff --git a/builtin/am.c b/builtin/am.c > index c973bd96dc..2f89338ed7 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state) > ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); > > if (!ret) { > - FREE_AND_NULL(state->msg); > + free(state->msg); > + state->msg = NULL; > if (read_commit_msg(state) < 0) > die(_("'%s' was deleted by the applypatch-msg hook"), > am_path(state, "final-commit")); > > So it looks to me like removing the "type T" rule breaks a lot of > things, but that the change you made to the expression rule is good, and > we should do that for the "type" rule as well. Your commit says the > "expression rule already covers it", but this doesn't seem to be the > case at all. > > As an aside: Junio, did you mean to apply f8bb4631fb to next this way? > Looks like a mis-applied scissor commit. I can't reproduce that strange result with master, but get a scissors line into the commit message as well. The diff at the end looks reasonable (contains just the manual stuff). Here's what I did: $ uname -a Linux debian 4.9.0-3-amd64 #1 SMP Debian 4.9.30-2 (2017-06-12) x86_64 GNU/Linux $ spatch -version spatch version 1.0.4 with Python support and with PCRE support $ git checkout -b cocci-free-null origin/master Branch cocci-free-null set up to track remote branch master from origin. Switched to a new branch 'cocci-free-null' $ git am # pasted my email Applying: coccinelle: add a rule to make "expression" code use FREE_AND_NULL() # results in commit message with scissor line, interesting.. $ git revert --no-edit 88ce3ef636 e140f7afdd 6a83d90207 [cocci-free-null b50d5eca5f] Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" 12 files changed, 49 insertions(+), 23 deletions(-) [cocci-free-null cf9072ce4e] Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" 2 files changed, 4 insertions(+), 2 deletions(-) [cocci-free-null 97675d4dc2] Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" 37 files changed, 142 insertions(+), 71 deletions(-) $ make contrib/coccinelle/free.cocci.patch GIT_VERSION = 2.13.2.537.g97675d4dc2 SPATCH contrib/coccinelle/free.cocci SPATCH result: contrib/coccinelle/free.cocci.patch $ patch -p0 author_name); + FREE_AND_NULL(state->author_email); + FREE_AND_NULL(state->author_date); + FREE_AND_NULL(state->msg); state->msg_len = 0; diff --git a/builtin/worktree.c b/builtin/worktree.c index c98e2ce5f5..0c5476ee9d 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -300,8 +300,10 @@ static int add_worktree(const char *path, const char *refname, } is_junk = 0; - FREE_AND_NULL(junk_work_tree); - FREE_AND_NULL(junk_git_dir); + free(junk_work_tree); + free(junk_git_dir); + junk_work_tree = NULL; + junk_git_dir = NULL; done: if (ret || !opts->keep_locked) { diff --git a/commit-slab.h b/commit-slab.h index 333d81e370..42d16dcded 100644 --- a/commit-slab.h +++ b/commit-slab.h @@ -82,7 +82,8 @@ static MAYBE_UNUSED void clear_ ##slabname(struct slabname *s) \ for (i = 0; i < s->slab_count; i++) \ free(s->slab[i]); \ s->slab_count = 0; \ - FREE_AND_NULL(s->slab); \ + free(s->slab); \ + s->slab = NULL; \ } \ \ static MAYBE_UNUSED elemtype *slabname## _at_peek(struct slabname *s, \ diff --git a/gpg-interface.c b/gpg-interface.c index d936f3a32f..8ab32df457 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -14,11 +14,16 @@ static const char *gpg_program = "gpg"; void signature_check_clear(struct signature_check *sigc) { - FREE_AND_NULL(sigc->payload); - FREE_AND_NULL(sigc->gpg_output); - FREE_AND_NULL(sigc->gpg_status); - FREE_AND_NULL(sigc->signer); - FREE_AND_NULL(sigc->key); + free(sigc->payload); + free(sigc->gpg_output); + free(sigc->gpg_status); + free(sigc->signer); + free(sigc->key); + sigc->payload = NULL; + sigc->gpg_output = NULL; + sigc->gpg_status = NULL; + sigc->signer = NULL; + sigc->key = NULL; } static struct { diff --git a/help.c b/help.c index 88a3aeaeb9..5af5319caf 100644 --- a/help.c +++ b/help.c @@ -269,8 +269,9 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) for (i = 0; i < old->cnt; i++) cmds->names[cmds->cnt++] = old->names[i]; - FREE_AND_NULL(old->names); + free(old->names); old->cnt = 0; + old->names = NULL; } /* An empirically derived magic number */ diff --git a/line-log.c b/line-log.c index ab0709f9ae..f225447ebb 100644 --- a/line-log.c +++ b/line-log.c @@ -34,8 +34,9 @@ void range_set_init(struct range_set *rs, size_t prealloc) void range_set_release(struct range_set *rs) { - FREE_AND_NULL(rs->ranges); + free(rs->ranges); rs->alloc = rs->nr = 0; + rs->ranges = NULL; } /* dst must be uninitialized! */ diff --git a/prio-queue.c b/prio-queue.c index 126d096727..fc3860fdcb 100644 --- a/prio-queue.c +++ b/prio-queue.c @@ -27,9 +27,10 @@ void prio_queue_reverse(struct prio_queue *queue) void clear_prio_queue(struct prio_queue *queue) { - FREE_AND_NULL(queue->array); + free(queue->array); queue->nr = 0; queue->alloc = 0; + queue->array = NULL; queue->insertion_ctr = 0; } diff --git a/refs/ref-cache.c b/refs/ref-cache.c index 76bb723c86..af2fcb2c12 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -82,8 +82,9 @@ static void clear_ref_dir(struct ref_dir *dir) int i; for (i = 0; i < dir->nr; i++) free_ref_entry(dir->entries[i]); - FREE_AND_NULL(dir->entries); + free(dir->entries); dir->sorted = dir->nr = dir->alloc = 0; + dir->entries = NULL; } struct ref_entry *create_dir_entry(struct ref_cache *cache, diff --git a/rerere.c b/rerere.c index 70634d456c..829b3b0f08 100644 --- a/rerere.c +++ b/rerere.c @@ -40,8 +40,9 @@ static void free_rerere_dirs(void) free(rerere_dir[i]->status); free(rerere_dir[i]); } - FREE_AND_NULL(rerere_dir); + free(rerere_dir); rerere_dir_nr = rerere_dir_alloc = 0; + rerere_dir = NULL; } static void free_rerere_id(struct string_list_item *item) diff --git a/split-index.c b/split-index.c index 83e39ec8d7..f519e60f87 100644 --- a/split-index.c +++ b/split-index.c @@ -167,9 +167,10 @@ void merge_base_index(struct index_state *istate) ewah_free(si->delete_bitmap); ewah_free(si->replace_bitmap); - FREE_AND_NULL(si->saved_cache); + free(si->saved_cache); si->delete_bitmap = NULL; si->replace_bitmap = NULL; + si->saved_cache = NULL; si->saved_cache_nr = 0; } ``` ## Ævar Arnfjörð Bjarmason, 2017-06-28 22:30 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: <87efu3kaog.fsf@gmail.com> URL: https://gitlist.dev/e/87efu3kaog.fsf%40gmail.com In-Reply-To: <433f9fb7-a202-1091-46de-04535b2a49e1@web.de> ``` On Wed, Jun 28 2017, René Scharfe jotted: > Am 28.06.2017 um 23:39 schrieb Ævar Arnfjörð Bjarmason: >> >> On Sun, Jun 25 2017, René Scharfe jotted: >> >>> Am 16.06.2017 um 21:43 schrieb Junio C Hamano: >>>> Ævar Arnfjörð Bjarmason writes: >>>> >>>>> A follow-up to the existing "type" rule added in an earlier >>>>> change. This catches some occurrences that are missed by the previous >>>>> rule. >>>>> >>>>> Signed-off-by: Ævar Arnfjörð Bjarmason >>>>> --- >>>> >>>> Hmph, I wonder if the "type" thing is really needed. Over there, >>>> "ptr" is an expression and we can find "free(ptr); ptr = NULL" with >>>> the rule in this patch already, no? >>> >>> Indeed. How about this on top of master? >>> >>> -- >8 -- >>> Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules >>> >>> There are two rules for using FREE_AND_NULL in free.cocci, one for >>> pointer types and one for expressions. Both cause coccinelle to remove >>> empty lines and even newline characters between replacements for some >>> reason; consecutive "free(x);/x=NULL;" sequences end up as multiple >>> FREE_AND_NULL calls on the same time. >>> >>> Remove the type rule, as the expression rule already covers it, and >>> rearrange the lines of the latter to place the addition of FREE_AND_NULL >>> between the removals, which causes coccinelle to leave surrounding >>> whitespace untouched. >>> >>> Signed-off-by: Rene Scharfe >>> --- >>> contrib/coccinelle/free.cocci | 10 +--------- >>> 1 file changed, 1 insertion(+), 9 deletions(-) >>> >>> diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci >>> index f2d97e755b..4490069df9 100644 >>> --- a/contrib/coccinelle/free.cocci >>> +++ b/contrib/coccinelle/free.cocci >>> @@ -11,16 +11,8 @@ expression E; >>> free(E); >>> >>> @@ >>> -type T; >>> -T *ptr; >>> -@@ >>> -- free(ptr); >>> -- ptr = NULL; >>> -+ FREE_AND_NULL(ptr); >>> - >>> -@@ >>> expression E; >>> @@ >>> - free(E); >>> -- E = NULL; >>> + FREE_AND_NULL(E); >>> +- E = NULL; >> >> Late reply, sorry. What version of coccinelle are you running? I have >> 1.0.4 (from Debian) and can't get this to produce the same results as >> what I have. >> >> On top of next, I did: >> >> Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" >> Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" >> Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" >> >> And then generated the patch as usual with `make coccicheck`, and >> applied it. This has your change. >> >> I then re-applied the manual "*.[ch] refactoring" changes >> >> This results in this diff with next: >> >> $ git diff --stat origin/next.. -- '*.[ch]' >> builtin/am.c | 3 ++- >> builtin/clean.c | 6 ++++-- >> builtin/config.c | 6 ++++-- >> builtin/index-pack.c | 6 ++++-- >> builtin/pack-objects.c | 12 ++++++++---- >> builtin/unpack-objects.c | 3 ++- >> fast-import.c | 6 ++++-- >> http-push.c | 24 ++++++++++++++++-------- >> http.c | 15 ++++++++++----- >> imap-send.c | 3 ++- >> ref-filter.c | 3 ++- >> refs/files-backend.c | 3 ++- >> remote-testsvn.c | 3 ++- >> sequencer.c | 3 ++- >> sha1-array.c | 3 ++- >> sha1_file.c | 3 ++- >> transport-helper.c | 27 ++++++++++++++++++--------- >> transport.c | 3 ++- >> tree-diff.c | 6 ++++-- >> tree.c | 3 ++- >> 20 files changed, 94 insertions(+), 47 deletions(-) >> >> These are all cases where we now miss things that should use >> FREE_AND_NULL(), e.g.: >> >> diff --git a/builtin/am.c b/builtin/am.c >> index c973bd96dc..2f89338ed7 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state) >> ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); >> >> if (!ret) { >> - FREE_AND_NULL(state->msg); >> + free(state->msg); >> + state->msg = NULL; >> if (read_commit_msg(state) < 0) >> die(_("'%s' was deleted by the applypatch-msg hook"), >> am_path(state, "final-commit")); >> >> So it looks to me like removing the "type T" rule breaks a lot of >> things, but that the change you made to the expression rule is good, and >> we should do that for the "type" rule as well. Your commit says the >> "expression rule already covers it", but this doesn't seem to be the >> case at all. >> >> As an aside: Junio, did you mean to apply f8bb4631fb to next this way? >> Looks like a mis-applied scissor commit. > > I can't reproduce that strange result with master, but get a scissors > line into the commit message as well. The diff at the end looks > reasonable (contains just the manual stuff). Here's what I did: I get exactly the same thing as you do below when following these steps. So it seems your patch in 2122b01f-7627-cd1b-c7df-751c076f90ff@web.de is just fine as-is and I just screwed something up when testing this. Sorry about the noise. Since this works your original patch is obviously preferrable since it's not duplicating the rule. > $ uname -a > Linux debian 4.9.0-3-amd64 #1 SMP Debian 4.9.30-2 (2017-06-12) x86_64 GNU/Linux > > $ spatch -version > spatch version 1.0.4 with Python support and with PCRE support > > $ git checkout -b cocci-free-null origin/master > Branch cocci-free-null set up to track remote branch master from origin. > Switched to a new branch 'cocci-free-null' > > $ git am > # pasted my email > Applying: coccinelle: add a rule to make "expression" code use FREE_AND_NULL() > # results in commit message with scissor line, interesting.. > > $ git revert --no-edit 88ce3ef636 e140f7afdd 6a83d90207 > [cocci-free-null b50d5eca5f] Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" > 12 files changed, 49 insertions(+), 23 deletions(-) > [cocci-free-null cf9072ce4e] Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" > 2 files changed, 4 insertions(+), 2 deletions(-) > [cocci-free-null 97675d4dc2] Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" > 37 files changed, 142 insertions(+), 71 deletions(-) > > $ make contrib/coccinelle/free.cocci.patch > GIT_VERSION = 2.13.2.537.g97675d4dc2 > SPATCH contrib/coccinelle/free.cocci > SPATCH result: contrib/coccinelle/free.cocci.patch > > $ patch -p0 patching file alias.c > patching file apply.c > patching file attr.c > patching file blame.c > patching file branch.c > patching file commit.c > patching file config.c > patching file credential.c > patching file diffcore-rename.c > patching file diff-lib.c > patching file diff.c > patching file dir.c > patching file grep.c > patching file line-log.c > patching file ll-merge.c > patching file mailinfo.c > patching file object.c > patching file pathspec.c > patching file read-cache.c > patching file refs/files-backend.c > patching file ref-filter.c > patching file sequencer.c > patching file sha1-array.c > patching file sha1_file.c > patching file transport.c > patching file transport-helper.c > patching file tree-diff.c > patching file tree.c > patching file builtin/am.c > patching file builtin/clean.c > patching file builtin/config.c > patching file builtin/index-pack.c > patching file builtin/pack-objects.c > patching file builtin/unpack-objects.c > patching file fast-import.c > patching file imap-send.c > patching file remote-testsvn.c > patching file http-push.c > patching file http.c > > $ git diff HEAD^^^ > diff --git a/builtin/am.c b/builtin/am.c > index c973bd96dc..eb24eb7b08 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1074,8 +1074,11 @@ static void am_next(struct am_state *state) > struct object_id head; > > FREE_AND_NULL(state->author_name); > + > FREE_AND_NULL(state->author_email); > + > FREE_AND_NULL(state->author_date); > + > FREE_AND_NULL(state->msg); > state->msg_len = 0; > > diff --git a/builtin/worktree.c b/builtin/worktree.c > index c98e2ce5f5..0c5476ee9d 100644 > --- a/builtin/worktree.c > +++ b/builtin/worktree.c > @@ -300,8 +300,10 @@ static int add_worktree(const char *path, const char *refname, > } > > is_junk = 0; > - FREE_AND_NULL(junk_work_tree); > - FREE_AND_NULL(junk_git_dir); > + free(junk_work_tree); > + free(junk_git_dir); > + junk_work_tree = NULL; > + junk_git_dir = NULL; > > done: > if (ret || !opts->keep_locked) { > diff --git a/commit-slab.h b/commit-slab.h > index 333d81e370..42d16dcded 100644 > --- a/commit-slab.h > +++ b/commit-slab.h > @@ -82,7 +82,8 @@ static MAYBE_UNUSED void clear_ ##slabname(struct slabname *s) \ > for (i = 0; i < s->slab_count; i++) \ > free(s->slab[i]); \ > s->slab_count = 0; \ > - FREE_AND_NULL(s->slab); \ > + free(s->slab); \ > + s->slab = NULL; \ > } \ > \ > static MAYBE_UNUSED elemtype *slabname## _at_peek(struct slabname *s, \ > diff --git a/gpg-interface.c b/gpg-interface.c > index d936f3a32f..8ab32df457 100644 > --- a/gpg-interface.c > +++ b/gpg-interface.c > @@ -14,11 +14,16 @@ static const char *gpg_program = "gpg"; > > void signature_check_clear(struct signature_check *sigc) > { > - FREE_AND_NULL(sigc->payload); > - FREE_AND_NULL(sigc->gpg_output); > - FREE_AND_NULL(sigc->gpg_status); > - FREE_AND_NULL(sigc->signer); > - FREE_AND_NULL(sigc->key); > + free(sigc->payload); > + free(sigc->gpg_output); > + free(sigc->gpg_status); > + free(sigc->signer); > + free(sigc->key); > + sigc->payload = NULL; > + sigc->gpg_output = NULL; > + sigc->gpg_status = NULL; > + sigc->signer = NULL; > + sigc->key = NULL; > } > > static struct { > diff --git a/help.c b/help.c > index 88a3aeaeb9..5af5319caf 100644 > --- a/help.c > +++ b/help.c > @@ -269,8 +269,9 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old) > > for (i = 0; i < old->cnt; i++) > cmds->names[cmds->cnt++] = old->names[i]; > - FREE_AND_NULL(old->names); > + free(old->names); > old->cnt = 0; > + old->names = NULL; > } > > /* An empirically derived magic number */ > diff --git a/line-log.c b/line-log.c > index ab0709f9ae..f225447ebb 100644 > --- a/line-log.c > +++ b/line-log.c > @@ -34,8 +34,9 @@ void range_set_init(struct range_set *rs, size_t prealloc) > > void range_set_release(struct range_set *rs) > { > - FREE_AND_NULL(rs->ranges); > + free(rs->ranges); > rs->alloc = rs->nr = 0; > + rs->ranges = NULL; > } > > /* dst must be uninitialized! */ > diff --git a/prio-queue.c b/prio-queue.c > index 126d096727..fc3860fdcb 100644 > --- a/prio-queue.c > +++ b/prio-queue.c > @@ -27,9 +27,10 @@ void prio_queue_reverse(struct prio_queue *queue) > > void clear_prio_queue(struct prio_queue *queue) > { > - FREE_AND_NULL(queue->array); > + free(queue->array); > queue->nr = 0; > queue->alloc = 0; > + queue->array = NULL; > queue->insertion_ctr = 0; > } > > diff --git a/refs/ref-cache.c b/refs/ref-cache.c > index 76bb723c86..af2fcb2c12 100644 > --- a/refs/ref-cache.c > +++ b/refs/ref-cache.c > @@ -82,8 +82,9 @@ static void clear_ref_dir(struct ref_dir *dir) > int i; > for (i = 0; i < dir->nr; i++) > free_ref_entry(dir->entries[i]); > - FREE_AND_NULL(dir->entries); > + free(dir->entries); > dir->sorted = dir->nr = dir->alloc = 0; > + dir->entries = NULL; > } > > struct ref_entry *create_dir_entry(struct ref_cache *cache, > diff --git a/rerere.c b/rerere.c > index 70634d456c..829b3b0f08 100644 > --- a/rerere.c > +++ b/rerere.c > @@ -40,8 +40,9 @@ static void free_rerere_dirs(void) > free(rerere_dir[i]->status); > free(rerere_dir[i]); > } > - FREE_AND_NULL(rerere_dir); > + free(rerere_dir); > rerere_dir_nr = rerere_dir_alloc = 0; > + rerere_dir = NULL; > } > > static void free_rerere_id(struct string_list_item *item) > diff --git a/split-index.c b/split-index.c > index 83e39ec8d7..f519e60f87 100644 > --- a/split-index.c > +++ b/split-index.c > @@ -167,9 +167,10 @@ void merge_base_index(struct index_state *istate) > > ewah_free(si->delete_bitmap); > ewah_free(si->replace_bitmap); > - FREE_AND_NULL(si->saved_cache); > + free(si->saved_cache); > si->delete_bitmap = NULL; > si->replace_bitmap = NULL; > + si->saved_cache = NULL; > si->saved_cache_nr = 0; > } > ``` ## René Scharfe, 2017-06-28 22:30 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: URL: https://gitlist.dev/e/b1b97b26-f715-1d0f-410e-51356b05b890%40web.de In-Reply-To: <433f9fb7-a202-1091-46de-04535b2a49e1@web.de> ``` Am 29.06.2017 um 00:21 schrieb René Scharfe: > Am 28.06.2017 um 23:39 schrieb Ævar Arnfjörð Bjarmason: >> >> On Sun, Jun 25 2017, René Scharfe jotted: >> >>> Am 16.06.2017 um 21:43 schrieb Junio C Hamano: >>>> Ævar Arnfjörð Bjarmason writes: >>>> >>>>> A follow-up to the existing "type" rule added in an earlier >>>>> change. This catches some occurrences that are missed by the previous >>>>> rule. >>>>> >>>>> Signed-off-by: Ævar Arnfjörð Bjarmason >>>>> --- >>>> >>>> Hmph, I wonder if the "type" thing is really needed. Over there, >>>> "ptr" is an expression and we can find "free(ptr); ptr = NULL" with >>>> the rule in this patch already, no? >>> >>> Indeed. How about this on top of master? >>> >>> -- >8 -- >>> Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules >>> >>> There are two rules for using FREE_AND_NULL in free.cocci, one for >>> pointer types and one for expressions. Both cause coccinelle to remove >>> empty lines and even newline characters between replacements for some >>> reason; consecutive "free(x);/x=NULL;" sequences end up as multiple >>> FREE_AND_NULL calls on the same time. >>> >>> Remove the type rule, as the expression rule already covers it, and >>> rearrange the lines of the latter to place the addition of FREE_AND_NULL >>> between the removals, which causes coccinelle to leave surrounding >>> whitespace untouched. >>> >>> Signed-off-by: Rene Scharfe >>> --- >>> contrib/coccinelle/free.cocci | 10 +--------- >>> 1 file changed, 1 insertion(+), 9 deletions(-) >>> >>> diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci >>> index f2d97e755b..4490069df9 100644 >>> --- a/contrib/coccinelle/free.cocci >>> +++ b/contrib/coccinelle/free.cocci >>> @@ -11,16 +11,8 @@ expression E; >>> free(E); >>> >>> @@ >>> -type T; >>> -T *ptr; >>> -@@ >>> -- free(ptr); >>> -- ptr = NULL; >>> -+ FREE_AND_NULL(ptr); >>> - >>> -@@ >>> expression E; >>> @@ >>> - free(E); >>> -- E = NULL; >>> + FREE_AND_NULL(E); >>> +- E = NULL; >> >> Late reply, sorry. What version of coccinelle are you running? I have >> 1.0.4 (from Debian) and can't get this to produce the same results as >> what I have. >> >> On top of next, I did: >> >> Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro" >> Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule" >> Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule" >> >> And then generated the patch as usual with `make coccicheck`, and >> applied it. This has your change. >> >> I then re-applied the manual "*.[ch] refactoring" changes >> >> This results in this diff with next: >> >> $ git diff --stat origin/next.. -- '*.[ch]' >> builtin/am.c | 3 ++- >> builtin/clean.c | 6 ++++-- >> builtin/config.c | 6 ++++-- >> builtin/index-pack.c | 6 ++++-- >> builtin/pack-objects.c | 12 ++++++++---- >> builtin/unpack-objects.c | 3 ++- >> fast-import.c | 6 ++++-- >> http-push.c | 24 ++++++++++++++++-------- >> http.c | 15 ++++++++++----- >> imap-send.c | 3 ++- >> ref-filter.c | 3 ++- >> refs/files-backend.c | 3 ++- >> remote-testsvn.c | 3 ++- >> sequencer.c | 3 ++- >> sha1-array.c | 3 ++- >> sha1_file.c | 3 ++- >> transport-helper.c | 27 ++++++++++++++++++--------- >> transport.c | 3 ++- >> tree-diff.c | 6 ++++-- >> tree.c | 3 ++- >> 20 files changed, 94 insertions(+), 47 deletions(-) >> >> These are all cases where we now miss things that should use >> FREE_AND_NULL(), e.g.: >> >> diff --git a/builtin/am.c b/builtin/am.c >> index c973bd96dc..2f89338ed7 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state) >> ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); >> >> if (!ret) { >> - FREE_AND_NULL(state->msg); >> + free(state->msg); >> + state->msg = NULL; >> if (read_commit_msg(state) < 0) >> die(_("'%s' was deleted by the applypatch-msg hook"), >> am_path(state, "final-commit")); >> >> So it looks to me like removing the "type T" rule breaks a lot of >> things, but that the change you made to the expression rule is good, and >> we should do that for the "type" rule as well. Your commit says the >> "expression rule already covers it", but this doesn't seem to be the >> case at all. >> >> As an aside: Junio, did you mean to apply f8bb4631fb to next this way? >> Looks like a mis-applied scissor commit. > > I can't reproduce that strange result with master, but get a scissors > line into the commit message as well. The diff at the end looks > reasonable (contains just the manual stuff). Here's what I did: On top of next it looks basically the same for me, only difference being several new converted cases in repository.c. Did you commit before diff? René ``` ## Ævar Arnfjörð Bjarmason, 2017-06-28 22:35 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: URL: https://gitlist.dev/e/CACBZZX6XoAKEbxgStcZCo30ahZ5NiG4oKp7TJUYJFYTXb%3DEXGw%40mail.gmail.com In-Reply-To: ``` On Thu, Jun 29, 2017 at 12:30 AM, René Scharfe wrote: > Am 29.06.2017 um 00:21 schrieb René Scharfe: >> >> Am 28.06.2017 um 23:39 schrieb Ævar Arnfjörð Bjarmason: >>> >>> >>> On Sun, Jun 25 2017, René Scharfe jotted: >>> >>>> Am 16.06.2017 um 21:43 schrieb Junio C Hamano: >>>>> >>>>> Ævar Arnfjörð Bjarmason writes: >>>>> >>>>>> A follow-up to the existing "type" rule added in an earlier >>>>>> change. This catches some occurrences that are missed by the previous >>>>>> rule. >>>>>> >>>>>> Signed-off-by: Ævar Arnfjörð Bjarmason >>>>>> --- >>>>> >>>>> >>>>> Hmph, I wonder if the "type" thing is really needed. Over there, >>>>> "ptr" is an expression and we can find "free(ptr); ptr = NULL" with >>>>> the rule in this patch already, no? >>>> >>>> >>>> Indeed. How about this on top of master? >>>> >>>> -- >8 -- >>>> Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules >>>> >>>> There are two rules for using FREE_AND_NULL in free.cocci, one for >>>> pointer types and one for expressions. Both cause coccinelle to remove >>>> empty lines and even newline characters between replacements for some >>>> reason; consecutive "free(x);/x=NULL;" sequences end up as multiple >>>> FREE_AND_NULL calls on the same time. >>>> >>>> Remove the type rule, as the expression rule already covers it, and >>>> rearrange the lines of the latter to place the addition of FREE_AND_NULL >>>> between the removals, which causes coccinelle to leave surrounding >>>> whitespace untouched. >>>> >>>> Signed-off-by: Rene Scharfe >>>> --- >>>> contrib/coccinelle/free.cocci | 10 +--------- >>>> 1 file changed, 1 insertion(+), 9 deletions(-) >>>> >>>> diff --git a/contrib/coccinelle/free.cocci >>>> b/contrib/coccinelle/free.cocci >>>> index f2d97e755b..4490069df9 100644 >>>> --- a/contrib/coccinelle/free.cocci >>>> +++ b/contrib/coccinelle/free.cocci >>>> @@ -11,16 +11,8 @@ expression E; >>>> free(E); >>>> >>>> @@ >>>> -type T; >>>> -T *ptr; >>>> -@@ >>>> -- free(ptr); >>>> -- ptr = NULL; >>>> -+ FREE_AND_NULL(ptr); >>>> - >>>> -@@ >>>> expression E; >>>> @@ >>>> - free(E); >>>> -- E = NULL; >>>> + FREE_AND_NULL(E); >>>> +- E = NULL; >>> >>> >>> Late reply, sorry. What version of coccinelle are you running? I have >>> 1.0.4 (from Debian) and can't get this to produce the same results as >>> what I have. >>> >>> On top of next, I did: >>> >>> Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() >>> macro" >>> Revert "coccinelle: make use of the "expression" >>> FREE_AND_NULL() rule" >>> Revert "coccinelle: make use of the "type" FREE_AND_NULL() >>> rule" >>> >>> And then generated the patch as usual with `make coccicheck`, and >>> applied it. This has your change. >>> >>> I then re-applied the manual "*.[ch] refactoring" changes >>> >>> This results in this diff with next: >>> >>> $ git diff --stat origin/next.. -- '*.[ch]' >>> builtin/am.c | 3 ++- >>> builtin/clean.c | 6 ++++-- >>> builtin/config.c | 6 ++++-- >>> builtin/index-pack.c | 6 ++++-- >>> builtin/pack-objects.c | 12 ++++++++---- >>> builtin/unpack-objects.c | 3 ++- >>> fast-import.c | 6 ++++-- >>> http-push.c | 24 ++++++++++++++++-------- >>> http.c | 15 ++++++++++----- >>> imap-send.c | 3 ++- >>> ref-filter.c | 3 ++- >>> refs/files-backend.c | 3 ++- >>> remote-testsvn.c | 3 ++- >>> sequencer.c | 3 ++- >>> sha1-array.c | 3 ++- >>> sha1_file.c | 3 ++- >>> transport-helper.c | 27 ++++++++++++++++++--------- >>> transport.c | 3 ++- >>> tree-diff.c | 6 ++++-- >>> tree.c | 3 ++- >>> 20 files changed, 94 insertions(+), 47 deletions(-) >>> >>> These are all cases where we now miss things that should use >>> FREE_AND_NULL(), e.g.: >>> >>> diff --git a/builtin/am.c b/builtin/am.c >>> index c973bd96dc..2f89338ed7 100644 >>> --- a/builtin/am.c >>> +++ b/builtin/am.c >>> @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct >>> am_state *state) >>> ret = run_hook_le(NULL, "applypatch-msg", am_path(state, >>> "final-commit"), NULL); >>> >>> if (!ret) { >>> - FREE_AND_NULL(state->msg); >>> + free(state->msg); >>> + state->msg = NULL; >>> if (read_commit_msg(state) < 0) >>> die(_("'%s' was deleted by the >>> applypatch-msg hook"), >>> am_path(state, "final-commit")); >>> >>> So it looks to me like removing the "type T" rule breaks a lot of >>> things, but that the change you made to the expression rule is good, and >>> we should do that for the "type" rule as well. Your commit says the >>> "expression rule already covers it", but this doesn't seem to be the >>> case at all. >>> >>> As an aside: Junio, did you mean to apply f8bb4631fb to next this way? >>> Looks like a mis-applied scissor commit. >> >> >> I can't reproduce that strange result with master, but get a scissors >> line into the commit message as well. The diff at the end looks >> reasonable (contains just the manual stuff). Here's what I did: > > > On top of next it looks basically the same for me, only difference > being several new converted cases in repository.c. > > Did you commit before diff? At this point I have no idea what I did wrong, and no time/want to really find out. But whatever it was was the wrong thing, obviously :) ``` ## René Scharfe, 2017-06-28 22:44 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: URL: https://gitlist.dev/e/f0fa8268-227c-829b-8611-2f94a5c4c1ee%40web.de In-Reply-To: <433f9fb7-a202-1091-46de-04535b2a49e1@web.de> ``` Am 29.06.2017 um 00:21 schrieb René Scharfe: > $ git am > # pasted my email > Applying: coccinelle: add a rule to make "expression" code use FREE_AND_NULL() > # results in commit message with scissor line, interesting.. "git am --scissors" commits with the correct message. I'll do a "git config --local --add mailinfo.scissors true" real quick.. René ``` ## Junio C Hamano, 2017-06-28 23:15 Subject: Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL() Message-ID: URL: https://gitlist.dev/e/xmqq8tkbvh3x.fsf%40gitster.mtv.corp.google.com In-Reply-To: <87efu3kaog.fsf@gmail.com> ``` Ævar Arnfjörð Bjarmason writes: > I get exactly the same thing as you do below when following these > steps. So it seems your patch in > 2122b01f-7627-cd1b-c7df-751c076f90ff@web.de is just fine as-is and I > just screwed something up when testing this. > > Sorry about the noise. Since this works your original patch is obviously > preferrable since it's not duplicating the rule. OK. Unfortunately I screwed up and merged the revert already to 'next'. So I'll queue René's original again to 'next' and we'll only have one rule at the end. Thanks, both. ```