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