{"thread":{"id":"12162","subject":"[PATCH] Remove useless if-before-free tests.","startedAt":"2008-02-17T21:58:49Z","lastAt":"2009-02-26T13:48:25Z","messageCount":18,"participants":["Jim Meyering","David Symonds","Johannes Schindelin","Junio C Hamano","Jean-Luc Herren","Morten Welinder","Uwe Kleine-König","Mike Ralphson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"69044","messageId":"871w7bz1ly.fsf@rho.meyering.net","threadId":"12162","inReplyTo":null,"subject":"[PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-17T21:58:49Z","receivedAt":"2008-02-17T21:58:49Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"This change removes all useless if-before-free tests.\nE.g., it replace code like this\n\n\tif (some_expression)\n\t\tfree (some_expression);\n\nwith the now-equivalent\n\n\tfree (some_expression);\n\nIt is equivalent not just because POSIX has required free(NULL)\nto work for a long time, but simply because it has worked for\nso long that no reasonable porting target fails the test.\nHere's some evidence from nearly 1.5 years ago:\n\n    http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n\nFYI, the change below was prepared by running the following:\n\n  git ls-files -z | xargs -0 \\\n  perl -0x3b -pi -e \\\n    's/\\bif\\s*\\(\\s*(\\S+?)(?:\\s*!=\\s*NULL)?\\s*\\)\\s+(free\\s*\\(\\s*\\1\\s*\\))/$2/s'\n\nNote however, that it doesn't handle brace-enclosed blocks like\n\"if (x) { free (x); }\".  But that's ok, since there were none like\nthat in git sources.\n\nBeware: if you do use the above snippet, note that it can\nproduce syntactically invalid C code.  That happens when the\naffected \"if\"-statement has a matching \"else\".\nE.g., it would transform this\n\n  if (x)\n    free (x);\n  else\n    foo ();\n\ninto this:\n\n  free (x);\n  else\n    foo ();\n\nThere were none of those here, either.\n\nIf you're interested in automating detection of the useless\ntests, you might like the useless-if-before-free script in gnulib:\n[it *does* detect brace-enclosed free statements, and has a --name=S\n option to make it detect free-like functions with different names]\n\n  http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=build-aux/useless-if-before-free\n\nI confirmed that \"make test\" passes with this change.\n\nSigned-off-by: Jim Meyering <meyering@redhat.com>\n---\n builtin-blame.c        |    3 +--\n builtin-branch.c       |    9 +++------\n builtin-fast-export.c  |    3 +--\n builtin-http-fetch.c   |    3 +--\n builtin-pack-objects.c |    3 +--\n builtin-revert.c       |    3 +--\n connect.c              |    3 +--\n diff.c                 |    9 +++------\n dir.c                  |    3 +--\n http-push.c            |   18 ++++++------------\n imap-send.c            |    3 +--\n interpolate.c          |    3 +--\n pretty.c               |    3 +--\n remote.c               |    3 +--\n setup.c                |    3 +--\n sha1_name.c            |    6 ++----\n xdiff-interface.c      |    3 +--\n 17 files changed, 27 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 9b4c02e..cd12a84 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -123,8 +123,7 @@ static inline struct origin *origin_incref(struct origin *o)\n static void origin_decref(struct origin *o)\n {\n \tif (o && --o->refcnt <= 0) {\n-\t\tif (o->file.ptr)\n-\t\t\tfree(o->file.ptr);\n+\t\tfree(o->file.ptr);\n \t\tfree(o);\n \t}\n }\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 089cae5..e75a425 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -123,8 +123,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t\tcontinue;\n \t\t}\n\n-\t\tif (name)\n-\t\t\tfree(name);\n+\t\tfree(name);\n\n \t\tname = xstrdup(mkpath(fmt, argv[i]));\n \t\tif (!resolve_ref(name, sha1, 1, NULL)) {\n@@ -169,8 +168,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t}\n \t}\n\n-\tif (name)\n-\t\tfree(name);\n+\tfree(name);\n\n \treturn(ret);\n }\n@@ -487,8 +485,7 @@ static void create_branch(const char *name, const char *start_name,\n \tif (write_ref_sha1(lock, sha1, msg) < 0)\n \t\tdie(\"Failed to write ref: %s.\", strerror(errno));\n\n-\tif (real_ref)\n-\t\tfree(real_ref);\n+\tfree(real_ref);\n }\n\n static void rename_branch(const char *oldname, const char *newname, int force)\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex ef27eee..94ab967 100755\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -196,8 +196,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n \t\t\t  ? strlen(reencoded) : message\n \t\t\t  ? strlen(message) : 0),\n \t       reencoded ? reencoded : message ? message : \"\");\n-\tif (reencoded)\n-\t\tfree(reencoded);\n+\tfree(reencoded);\n\n \tfor (i = 0, p = commit->parents; p; p = p->next) {\n \t\tint mark = get_object_mark(&p->item->object);\ndiff --git a/builtin-http-fetch.c b/builtin-http-fetch.c\nindex 7f450c6..299093f 100644\n--- a/builtin-http-fetch.c\n+++ b/builtin-http-fetch.c\n@@ -80,8 +80,7 @@ int cmd_http_fetch(int argc, const char **argv, const char *prefix)\n\n \twalker_free(walker);\n\n-\tif (rewritten_url)\n-\t\tfree(rewritten_url);\n+\tfree(rewritten_url);\n\n \treturn rc;\n }\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 692a761..3f49205 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1428,8 +1428,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t * accounting lock.  Compiler will optimize the strangeness\n \t * away when THREADED_DELTA_SEARCH is not defined.\n \t */\n-\tif (trg_entry->delta_data)\n-\t\tfree(trg_entry->delta_data);\n+\tfree(trg_entry->delta_data);\n \tcache_lock();\n \tif (trg_entry->delta_data) {\n \t\tdelta_cache_size -= trg_entry->delta_size;\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 358af53..59b3c30 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -396,8 +396,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\telse\n \t\t\treturn execl_git_cmd(\"commit\", \"-n\", \"-F\", defmsg, NULL);\n \t}\n-\tif (reencoded_message)\n-\t\tfree(reencoded_message);\n+\tfree(reencoded_message);\n\n \treturn 0;\n }\ndiff --git a/connect.c b/connect.c\nindex 3aefd4a..29c74d4 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -68,8 +68,7 @@ struct ref **get_remote_heads(int in, struct ref **list,\n\n \t\tname_len = strlen(name);\n \t\tif (len != name_len + 41) {\n-\t\t\tif (server_capabilities)\n-\t\t\t\tfree(server_capabilities);\n+\t\t\tfree(server_capabilities);\n \t\t\tserver_capabilities = xstrdup(name + name_len + 1);\n \t\t}\n\ndiff --git a/diff.c b/diff.c\nindex 5b8afdc..6349eb1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -121,8 +121,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v\n \t\tpp->next = funcname_pattern_list;\n \t\tfuncname_pattern_list = pp;\n \t}\n-\tif (pp->pattern)\n-\t\tfree(pp->pattern);\n+\tfree(pp->pattern);\n \tpp->pattern = xstrdup(value);\n \treturn 0;\n }\n@@ -490,10 +489,8 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \t\t\t\tecbdata->diff_words->plus.text.size)\n \t\t\tdiff_words_show(ecbdata->diff_words);\n\n-\t\tif (ecbdata->diff_words->minus.text.ptr)\n-\t\t\tfree (ecbdata->diff_words->minus.text.ptr);\n-\t\tif (ecbdata->diff_words->plus.text.ptr)\n-\t\t\tfree (ecbdata->diff_words->plus.text.ptr);\n+\t\tfree (ecbdata->diff_words->minus.text.ptr);\n+\t\tfree (ecbdata->diff_words->plus.text.ptr);\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\ndiff --git a/dir.c b/dir.c\nindex 3e345c2..1514502 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -677,8 +677,7 @@ static struct path_simplify *create_simplify(const char **pathspec)\n\n static void free_simplify(struct path_simplify *simplify)\n {\n-\tif (simplify)\n-\t\tfree(simplify);\n+\tfree(simplify);\n }\n\n int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec)\ndiff --git a/http-push.c b/http-push.c\nindex b2b410d..5889f2e 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -664,8 +664,7 @@ static void release_request(struct transfer_request *request)\n \t\tclose(request->local_fileno);\n \tif (request->local_stream)\n \t\tfclose(request->local_stream);\n-\tif (request->url != NULL)\n-\t\tfree(request->url);\n+\tfree(request->url);\n \tfree(request);\n }\n\n@@ -1283,10 +1282,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \tstrbuf_release(&in_buffer);\n\n \tif (lock->token == NULL || lock->timeout <= 0) {\n-\t\tif (lock->token != NULL)\n-\t\t\tfree(lock->token);\n-\t\tif (lock->owner != NULL)\n-\t\t\tfree(lock->owner);\n+\t\tfree(lock->token);\n+\t\tfree(lock->owner);\n \t\tfree(url);\n \t\tfree(lock);\n \t\tlock = NULL;\n@@ -1344,8 +1341,7 @@ static int unlock_remote(struct remote_lock *lock)\n \t\t\tprev->next = prev->next->next;\n \t}\n\n-\tif (lock->owner != NULL)\n-\t\tfree(lock->owner);\n+\tfree(lock->owner);\n \tfree(lock->url);\n \tfree(lock->token);\n \tfree(lock);\n@@ -2028,8 +2024,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)\n \t}\n \tfree(url);\n\n-\tif (*symref != NULL)\n-\t\tfree(*symref);\n+\tfree(*symref);\n \t*symref = NULL;\n \thashclr(sha1);\n\n@@ -2425,8 +2420,7 @@ int main(int argc, char **argv)\n \t}\n\n  cleanup:\n-\tif (rewritten_url)\n-\t\tfree(rewritten_url);\n+\tfree(rewritten_url);\n \tif (info_ref_lock)\n \t\tunlock_remote(info_ref_lock);\n \tfree(remote);\ndiff --git a/imap-send.c b/imap-send.c\nindex a429a76..fb919ad 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -858,8 +858,7 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )\n \t\t  normal:\n \t\t\tif (cmdp->cb.done)\n \t\t\t\tcmdp->cb.done( ctx, cmdp, resp );\n-\t\t\tif (cmdp->cb.data)\n-\t\t\t\tfree( cmdp->cb.data );\n+\t\t\tfree( cmdp->cb.data );\n \t\t\tfree( cmdp->cmd );\n \t\t\tfree( cmdp );\n \t\t\tif (!tcmd || tcmd == cmdp)\ndiff --git a/interpolate.c b/interpolate.c\nindex 6ef53f2..7f03bd9 100644\n--- a/interpolate.c\n+++ b/interpolate.c\n@@ -11,8 +11,7 @@ void interp_set_entry(struct interp *table, int slot, const char *value)\n \tchar *oldval = table[slot].value;\n \tchar *newval = NULL;\n\n-\tif (oldval)\n-\t\tfree(oldval);\n+\tfree(oldval);\n\n \tif (value)\n \t\tnewval = xstrdup(value);\ndiff --git a/pretty.c b/pretty.c\nindex b987ff2..eca3ce1 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -30,8 +30,7 @@ enum cmit_fmt get_commit_format(const char *arg)\n \tif (*arg == '=')\n \t\targ++;\n \tif (!prefixcmp(arg, \"format:\")) {\n-\t\tif (user_format)\n-\t\t\tfree(user_format);\n+\t\tfree(user_format);\n \t\tuser_format = xstrdup(arg + 7);\n \t\treturn CMIT_FMT_USERFORMAT;\n \t}\ndiff --git a/remote.c b/remote.c\nindex 0e00680..b4cbed8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -493,8 +493,7 @@ void free_refs(struct ref *ref)\n \tstruct ref *next;\n \twhile (ref) {\n \t\tnext = ref->next;\n-\t\tif (ref->peer_ref)\n-\t\t\tfree(ref->peer_ref);\n+\t\tfree(ref->peer_ref);\n \t\tfree(ref);\n \t\tref = next;\n \t}\ndiff --git a/setup.c b/setup.c\nindex adede16..24c30d1 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -372,8 +372,7 @@ int check_repository_format_version(const char *var, const char *value)\n \t\tif (is_bare_repository_cfg == 1)\n \t\t\tinside_work_tree = -1;\n \t} else if (strcmp(var, \"core.worktree\") == 0) {\n-\t\tif (git_work_tree_cfg)\n-\t\t\tfree(git_work_tree_cfg);\n+\t\tfree(git_work_tree_cfg);\n \t\tgit_work_tree_cfg = xstrdup(value);\n \t\tinside_work_tree = -1;\n \t}\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 13e1164..5c3e60f 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -618,8 +618,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n\n \t\tcommit = pop_most_recent_commit(&list, ONELINE_SEEN);\n \t\tparse_object(commit->object.sha1);\n-\t\tif (temp_commit_buffer)\n-\t\t\tfree(temp_commit_buffer);\n+\t\tfree(temp_commit_buffer);\n \t\tif (commit->buffer)\n \t\t\tp = commit->buffer;\n \t\telse {\n@@ -636,8 +635,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n \t\t\tbreak;\n \t\t}\n \t}\n-\tif (temp_commit_buffer)\n-\t\tfree(temp_commit_buffer);\n+\tfree(temp_commit_buffer);\n \tfree_commit_list(list);\n \tfor (l = backup; l; l = l->next)\n \t\tclear_commit_marks(l->item, ONELINE_SEEN);\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex 4b8e5cc..bba2364 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -233,8 +233,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)\n \t\t\texpression = value;\n \t\tif (regcomp(&reg->re, expression, 0))\n \t\t\tdie(\"Invalid regexp to look for hunk header: %s\", expression);\n-\t\tif (buffer)\n-\t\t\tfree(buffer);\n+\t\tfree(buffer);\n \t\tvalue = ep + 1;\n \t}\n }\n--\n1.5.4.1.144.gc2249\n"},{"id":"69046","messageId":"ee77f5c20802171409k2dee2c87v8d84eba111c3d506@mail.gmail.com","threadId":"12162","inReplyTo":"871w7bz1ly.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2008-02-17T22:09:48Z","receivedAt":"2008-02-17T22:09:48Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On Feb 17, 2008 1:58 PM, Jim Meyering <jim@meyering.net> wrote:\n> This change removes all useless if-before-free tests.\n> E.g., it replace code like this\n>\n>         if (some_expression)\n>                 free (some_expression);\n>\n> with the now-equivalent\n>\n>         free (some_expression);\n>\n> It is equivalent not just because POSIX has required free(NULL)\n> to work for a long time, but simply because it has worked for\n> so long that no reasonable porting target fails the test.\n> Here's some evidence from nearly 1.5 years ago:\n>\n>     http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n\nThat's not great evidence. It only tests 9 systems, and misses several\ntargets that Git already runs on. It seems like a fairly minor cleanup\nfor a definite loss of portability.\n\nIt's also somewhat useful for indicating that the particular pointer\n*might* be NULL.\n\n\nDave.\n"},{"id":"69053","messageId":"alpine.LSU.1.00.0802172210470.30505@racer.site","threadId":"12162","inReplyTo":"871w7bz1ly.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-17T22:16:47Z","receivedAt":"2008-02-17T22:16:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 17 Feb 2008, Jim Meyering wrote:\n\n> It is equivalent not just because POSIX has required free(NULL) to work \n> for a long time, but simply because it has worked for so long that no \n> reasonable porting target fails the test. Here's some evidence from \n> nearly 1.5 years ago:\n> \n>     http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n\n>From this mail, we see that there is at least one target where this leads \nto a crash (remember, git should run on more platforms than Wine).\n\nHowever, such a crash is pretty obvious in our test-suite, I guess, and \nthus we could easily introduce something like this into git-compat-util.h \nshould the need ever arise:\n\n#ifdef FREE_NULL_CRASHES\ninline void gitfree(void *ptr)\n{\n\tif (ptr)\n\t\tfree(ptr);\n}\n#define free gitfree\n#endif\n\nIOW I like that type of cleanup.\n\nFWIW I tested MinGW (which is the only system I have access to that I \nsuspect of misbehaving), and it groks free(NULL) just fine.\n\nCiao,\nDscho\n"},{"id":"69107","messageId":"87ve4my6y2.fsf@rho.meyering.net","threadId":"12162","inReplyTo":"alpine.LSU.1.00.0802172210470.30505@racer.site","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-18T09:01:09Z","receivedAt":"2008-02-18T09:01:09Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> On Sun, 17 Feb 2008, Jim Meyering wrote:\n>\n>> It is equivalent not just because POSIX has required free(NULL) to work\n>> for a long time, but simply because it has worked for so long that no\n>> reasonable porting target fails the test. Here's some evidence from\n>> nearly 1.5 years ago:\n>>\n>>     http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n>\n>>From this mail, we see that there is at least one target where this leads\n> to a crash (remember, git should run on more platforms than Wine).\n\nHi,\n\nThanks for the feedback.\n\nFYI, you don't have to go back 20+ years to 3BSD to find a system on\nwhich free(NULL) fails :-)  SunOS4's did, too.  So if that is a reasonable\nporting target for git, then you will need the wrapper.  With references\nto \"SunOS\" in Makefile and configure, I did wonder about that.  Let me\nknow and I'll adjust the proposed patch.\n\n> However, such a crash is pretty obvious in our test-suite, I guess, and\n> thus we could easily introduce something like this into git-compat-util.h\n> should the need ever arise:\n>\n> #ifdef FREE_NULL_CRASHES\n> inline void gitfree(void *ptr)\n> {\n> \tif (ptr)\n> \t\tfree(ptr);\n> }\n> #define free gitfree\n> #endif\n>\n> IOW I like that type of cleanup.\n\n:-)\n"},{"id":"69109","messageId":"87pruuy64v.fsf@rho.meyering.net","threadId":"12162","inReplyTo":"ee77f5c20802171409k2dee2c87v8d84eba111c3d506@mail.gmail.com","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-18T09:18:40Z","receivedAt":"2008-02-18T09:18:40Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"\"David Symonds\" <dsymonds@gmail.com> wrote:\n> On Feb 17, 2008 1:58 PM, Jim Meyering <jim@meyering.net> wrote:\n>> This change removes all useless if-before-free tests.\n>> E.g., it replace code like this\n>>\n>>         if (some_expression)\n>>                 free (some_expression);\n>>\n>> with the now-equivalent\n>>\n>>         free (some_expression);\n>>\n>> It is equivalent not just because POSIX has required free(NULL)\n>> to work for a long time, but simply because it has worked for\n>> so long that no reasonable porting target fails the test.\n>> Here's some evidence from nearly 1.5 years ago:\n>>\n>>     http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n>\n> That's not great evidence. It only tests 9 systems, and misses several\n\nIf you mean mingw, cygwin, and M$-based ones, they're all ok.\nAs far as I know, you have to go back to SunOS4 to find a system on which\nfree(NULL) fails.  That OS stopped being a reasonable porting target\na couple years ago.\n\n> targets that Git already runs on. It seems like a fairly minor cleanup\n> for a definite loss of portability.\n\nIt's a definite loss of portability if you can find a reasonable porting\ntarget for which free(NULL) fails.  But even if you do, the fix is\nnot to reject the clean-up, but to amend it with a wrapper function.\nThat encapsulates the work-around in one place rather than polluting\nall of those files.\n"},{"id":"69111","messageId":"7vwsp2ppwc.fsf@gitster.siamese.dyndns.org","threadId":"12162","inReplyTo":"87pruuy64v.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-18T09:36:35Z","receivedAt":"2008-02-18T09:36:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> It's a definite loss of portability if you can find a reasonable porting\n> target for which free(NULL) fails.  But even if you do, the fix is\n> not to reject the clean-up, but to amend it with a wrapper function.\n> That encapsulates the work-around in one place rather than polluting\n> all of those files.\n\nAs we already have unchecked free(ptr) in our code _anyway_,\nthere _technically_ is no reason to reject the clean-up patch.\n\nWe just need to find a quiescent time to do so so that actively\ncooking patches in people's trees (and topics in 'next') won't\nget needless conflicts.\n"},{"id":"69153","messageId":"47B995CC.2000809@gmx.ch","threadId":"12162","inReplyTo":"871w7bz1ly.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jean-Luc Herren","fromEmail":"jlh@gmx.ch","sentAt":"2008-02-18T14:27:24Z","receivedAt":"2008-02-18T14:27:24Z","isPatch":true,"sender":{"key":"jlh@gmx.ch","avatar":null},"body":"Jim Meyering wrote:\n> This change removes all useless if-before-free tests.\n> E.g., it replace code like this\n> \n> \tif (some_expression)\n> \t\tfree (some_expression);\n> \n> with the now-equivalent\n> \n> \tfree (some_expression);\n\nWhile you're at it, you might want to add this to your patch:\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 9025d9a..3b27bca 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -472,7 +472,7 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,\n        if (socket_write( &imap->buf.sock, buf, bufl ) != bufl) {\n                free( cmd->cmd );\n                free( cmd );\n-               if (cb && cb->data)\n+               if (cb)\n                        free( cb->data );\n                return NULL;\n        }\n\njlh\n"},{"id":"69363","messageId":"87skznhqk6.fsf@rho.meyering.net","threadId":"12162","inReplyTo":"47B995CC.2000809@gmx.ch","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-20T10:26:17Z","receivedAt":"2008-02-20T10:26:17Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Jean-Luc Herren <jlh@gmx.ch> wrote:\n> Jim Meyering wrote:\n>> This change removes all useless if-before-free tests.\n...\n> While you're at it, you might want to add this to your patch:\n>\n> diff --git a/imap-send.c b/imap-send.c\n> index 9025d9a..3b27bca 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -472,7 +472,7 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,\n>        if (socket_write( &imap->buf.sock, buf, bufl ) != bufl) {\n>                free( cmd->cmd );\n>                free( cmd );\n> -               if (cb && cb->data)\n> +               if (cb)\n>                        free( cb->data );\n>                return NULL;\n>        }\n>\n\nWell spotted.\nHere's an updated patch:\n\n-----------------\nFrom 21ff212582df9b6b51028de99837b924840f3b58 Mon Sep 17 00:00:00 2001\nFrom: Jim Meyering <meyering@redhat.com>\nDate: Thu, 31 Jan 2008 18:26:32 +0100\nSubject: [PATCH] Avoid unnecessary \"if-before-free\" tests.\n\nThis change removes all obvious useless if-before-free tests.\nE.g., it replaces code like this:\n\n        if (some_expression)\n                free (some_expression);\n\nwith the now-equivalent:\n\n        free (some_expression);\n\nIt is equivalent not just because POSIX has required free(NULL)\nto work for a long time, but simply because it has worked for\nso long that no reasonable porting target fails the test.\nHere's some evidence from nearly 1.5 years ago:\n\n    http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n\nFYI, the change below was prepared by running the following:\n\n  git ls-files -z | xargs -0 \\\n  perl -0x3b -pi -e \\\n    's/\\bif\\s*\\(\\s*(\\S+?)(?:\\s*!=\\s*NULL)?\\s*\\)\\s+(free\\s*\\(\\s*\\1\\s*\\))/$2/s'\n\nNote however, that it doesn't handle brace-enclosed blocks like\n\"if (x) { free (x); }\".  But that's ok, since there were none like\nthat in git sources.\n\nBeware: if you do use the above snippet, note that it can\nproduce syntactically invalid C code.  That happens when the\naffected \"if\"-statement has a matching \"else\".\nE.g., it would transform this\n\n  if (x)\n    free (x);\n  else\n    foo ();\n\ninto this:\n\n  free (x);\n  else\n    foo ();\n\nThere were none of those here, either.\n\nIf you're interested in automating detection of the useless\ntests, you might like the useless-if-before-free script in gnulib:\n[it *does* detect brace-enclosed free statements, and has a --name=S\n option to make it detect free-like functions with different names]\n\n  http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=build-aux/useless-if-before-free\n\nAddendum:\n  Remove one more (in imap-send.c), spotted by Jean-Luc Herren <jlh@gmx.ch>.\n\nSigned-off-by: Jim Meyering <meyering@redhat.com>\n---\n builtin-blame.c        |    3 +--\n builtin-branch.c       |    9 +++------\n builtin-fast-export.c  |    3 +--\n builtin-http-fetch.c   |    3 +--\n builtin-pack-objects.c |    3 +--\n builtin-revert.c       |    3 +--\n connect.c              |    3 +--\n diff.c                 |    9 +++------\n dir.c                  |    3 +--\n http-push.c            |   18 ++++++------------\n imap-send.c            |    5 ++---\n interpolate.c          |    3 +--\n pretty.c               |    3 +--\n remote.c               |    3 +--\n setup.c                |    3 +--\n sha1_name.c            |    6 ++----\n xdiff-interface.c      |    3 +--\n 17 files changed, 28 insertions(+), 55 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 2d4a3e1..3b49217 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -123,8 +123,7 @@ static inline struct origin *origin_incref(struct origin *o)\n static void origin_decref(struct origin *o)\n {\n \tif (o && --o->refcnt <= 0) {\n-\t\tif (o->file.ptr)\n-\t\t\tfree(o->file.ptr);\n+\t\tfree(o->file.ptr);\n \t\tfree(o);\n \t}\n }\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex e414c88..93b78a7 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -126,8 +126,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t\tcontinue;\n \t\t}\n\n-\t\tif (name)\n-\t\t\tfree(name);\n+\t\tfree(name);\n\n \t\tname = xstrdup(mkpath(fmt, argv[i]));\n \t\tif (!resolve_ref(name, sha1, 1, NULL)) {\n@@ -172,8 +171,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t}\n \t}\n\n-\tif (name)\n-\t\tfree(name);\n+\tfree(name);\n\n \treturn(ret);\n }\n@@ -490,8 +488,7 @@ static void create_branch(const char *name, const char *start_name,\n \tif (write_ref_sha1(lock, sha1, msg) < 0)\n \t\tdie(\"Failed to write ref: %s.\", strerror(errno));\n\n-\tif (real_ref)\n-\t\tfree(real_ref);\n+\tfree(real_ref);\n }\n\n static void rename_branch(const char *oldname, const char *newname, int force)\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex f741df5..49b54de 100755\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -196,8 +196,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n \t\t\t  ? strlen(reencoded) : message\n \t\t\t  ? strlen(message) : 0),\n \t       reencoded ? reencoded : message ? message : \"\");\n-\tif (reencoded)\n-\t\tfree(reencoded);\n+\tfree(reencoded);\n\n \tfor (i = 0, p = commit->parents; p; p = p->next) {\n \t\tint mark = get_object_mark(&p->item->object);\ndiff --git a/builtin-http-fetch.c b/builtin-http-fetch.c\nindex 7f450c6..299093f 100644\n--- a/builtin-http-fetch.c\n+++ b/builtin-http-fetch.c\n@@ -80,8 +80,7 @@ int cmd_http_fetch(int argc, const char **argv, const char *prefix)\n\n \twalker_free(walker);\n\n-\tif (rewritten_url)\n-\t\tfree(rewritten_url);\n+\tfree(rewritten_url);\n\n \treturn rc;\n }\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex d2bb12e..7dff653 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1428,8 +1428,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t * accounting lock.  Compiler will optimize the strangeness\n \t * away when THREADED_DELTA_SEARCH is not defined.\n \t */\n-\tif (trg_entry->delta_data)\n-\t\tfree(trg_entry->delta_data);\n+\tfree(trg_entry->delta_data);\n \tcache_lock();\n \tif (trg_entry->delta_data) {\n \t\tdelta_cache_size -= trg_entry->delta_size;\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex e219859..b6dee6a 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -397,8 +397,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\telse\n \t\t\treturn execl_git_cmd(\"commit\", \"-n\", \"-F\", defmsg, NULL);\n \t}\n-\tif (reencoded_message)\n-\t\tfree(reencoded_message);\n+\tfree(reencoded_message);\n\n \treturn 0;\n }\ndiff --git a/connect.c b/connect.c\nindex 5ac3572..d12b105 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -68,8 +68,7 @@ struct ref **get_remote_heads(int in, struct ref **list,\n\n \t\tname_len = strlen(name);\n \t\tif (len != name_len + 41) {\n-\t\t\tif (server_capabilities)\n-\t\t\t\tfree(server_capabilities);\n+\t\t\tfree(server_capabilities);\n \t\t\tserver_capabilities = xstrdup(name + name_len + 1);\n \t\t}\n\ndiff --git a/diff.c b/diff.c\nindex 58fe775..21a81ab 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -118,8 +118,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v\n \t\tpp->next = funcname_pattern_list;\n \t\tfuncname_pattern_list = pp;\n \t}\n-\tif (pp->pattern)\n-\t\tfree(pp->pattern);\n+\tfree(pp->pattern);\n \tpp->pattern = xstrdup(value);\n \treturn 0;\n }\n@@ -492,10 +491,8 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \t\t\t\tecbdata->diff_words->plus.text.size)\n \t\t\tdiff_words_show(ecbdata->diff_words);\n\n-\t\tif (ecbdata->diff_words->minus.text.ptr)\n-\t\t\tfree (ecbdata->diff_words->minus.text.ptr);\n-\t\tif (ecbdata->diff_words->plus.text.ptr)\n-\t\t\tfree (ecbdata->diff_words->plus.text.ptr);\n+\t\tfree (ecbdata->diff_words->minus.text.ptr);\n+\t\tfree (ecbdata->diff_words->plus.text.ptr);\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\ndiff --git a/dir.c b/dir.c\nindex 1f507da..edc458e 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -704,8 +704,7 @@ static struct path_simplify *create_simplify(const char **pathspec)\n\n static void free_simplify(struct path_simplify *simplify)\n {\n-\tif (simplify)\n-\t\tfree(simplify);\n+\tfree(simplify);\n }\n\n int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec)\ndiff --git a/http-push.c b/http-push.c\nindex 63ff218..d122ed0 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -664,8 +664,7 @@ static void release_request(struct transfer_request *request)\n \t\tclose(request->local_fileno);\n \tif (request->local_stream)\n \t\tfclose(request->local_stream);\n-\tif (request->url != NULL)\n-\t\tfree(request->url);\n+\tfree(request->url);\n \tfree(request);\n }\n\n@@ -1283,10 +1282,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \tstrbuf_release(&in_buffer);\n\n \tif (lock->token == NULL || lock->timeout <= 0) {\n-\t\tif (lock->token != NULL)\n-\t\t\tfree(lock->token);\n-\t\tif (lock->owner != NULL)\n-\t\t\tfree(lock->owner);\n+\t\tfree(lock->token);\n+\t\tfree(lock->owner);\n \t\tfree(url);\n \t\tfree(lock);\n \t\tlock = NULL;\n@@ -1344,8 +1341,7 @@ static int unlock_remote(struct remote_lock *lock)\n \t\t\tprev->next = prev->next->next;\n \t}\n\n-\tif (lock->owner != NULL)\n-\t\tfree(lock->owner);\n+\tfree(lock->owner);\n \tfree(lock->url);\n \tfree(lock->token);\n \tfree(lock);\n@@ -2028,8 +2024,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)\n \t}\n \tfree(url);\n\n-\tif (*symref != NULL)\n-\t\tfree(*symref);\n+\tfree(*symref);\n \t*symref = NULL;\n \thashclr(sha1);\n\n@@ -2426,8 +2421,7 @@ int main(int argc, char **argv)\n \t}\n\n  cleanup:\n-\tif (rewritten_url)\n-\t\tfree(rewritten_url);\n+\tfree(rewritten_url);\n \tif (info_ref_lock)\n \t\tunlock_remote(info_ref_lock);\n \tfree(remote);\ndiff --git a/imap-send.c b/imap-send.c\nindex 9025d9a..10cce15 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -472,7 +472,7 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,\n \tif (socket_write( &imap->buf.sock, buf, bufl ) != bufl) {\n \t\tfree( cmd->cmd );\n \t\tfree( cmd );\n-\t\tif (cb && cb->data)\n+\t\tif (cb)\n \t\t\tfree( cb->data );\n \t\treturn NULL;\n \t}\n@@ -858,8 +858,7 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )\n \t\t  normal:\n \t\t\tif (cmdp->cb.done)\n \t\t\t\tcmdp->cb.done( ctx, cmdp, resp );\n-\t\t\tif (cmdp->cb.data)\n-\t\t\t\tfree( cmdp->cb.data );\n+\t\t\tfree( cmdp->cb.data );\n \t\t\tfree( cmdp->cmd );\n \t\t\tfree( cmdp );\n \t\t\tif (!tcmd || tcmd == cmdp)\ndiff --git a/interpolate.c b/interpolate.c\nindex 6ef53f2..7f03bd9 100644\n--- a/interpolate.c\n+++ b/interpolate.c\n@@ -11,8 +11,7 @@ void interp_set_entry(struct interp *table, int slot, const char *value)\n \tchar *oldval = table[slot].value;\n \tchar *newval = NULL;\n\n-\tif (oldval)\n-\t\tfree(oldval);\n+\tfree(oldval);\n\n \tif (value)\n \t\tnewval = xstrdup(value);\ndiff --git a/pretty.c b/pretty.c\nindex b987ff2..eca3ce1 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -30,8 +30,7 @@ enum cmit_fmt get_commit_format(const char *arg)\n \tif (*arg == '=')\n \t\targ++;\n \tif (!prefixcmp(arg, \"format:\")) {\n-\t\tif (user_format)\n-\t\t\tfree(user_format);\n+\t\tfree(user_format);\n \t\tuser_format = xstrdup(arg + 7);\n \t\treturn CMIT_FMT_USERFORMAT;\n \t}\ndiff --git a/remote.c b/remote.c\nindex 6b56473..ae1ef57 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -506,8 +506,7 @@ void free_refs(struct ref *ref)\n \tstruct ref *next;\n \twhile (ref) {\n \t\tnext = ref->next;\n-\t\tif (ref->peer_ref)\n-\t\t\tfree(ref->peer_ref);\n+\t\tfree(ref->peer_ref);\n \t\tfree(ref);\n \t\tref = next;\n \t}\ndiff --git a/setup.c b/setup.c\nindex 4509598..7917d7b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -374,8 +374,7 @@ int check_repository_format_version(const char *var, const char *value)\n \t} else if (strcmp(var, \"core.worktree\") == 0) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tif (git_work_tree_cfg)\n-\t\t\tfree(git_work_tree_cfg);\n+\t\tfree(git_work_tree_cfg);\n \t\tgit_work_tree_cfg = xstrdup(value);\n \t\tinside_work_tree = -1;\n \t}\ndiff --git a/sha1_name.c b/sha1_name.c\nindex ed3c867..cec2234 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -621,8 +621,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n\n \t\tcommit = pop_most_recent_commit(&list, ONELINE_SEEN);\n \t\tparse_object(commit->object.sha1);\n-\t\tif (temp_commit_buffer)\n-\t\t\tfree(temp_commit_buffer);\n+\t\tfree(temp_commit_buffer);\n \t\tif (commit->buffer)\n \t\t\tp = commit->buffer;\n \t\telse {\n@@ -639,8 +638,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n \t\t\tbreak;\n \t\t}\n \t}\n-\tif (temp_commit_buffer)\n-\t\tfree(temp_commit_buffer);\n+\tfree(temp_commit_buffer);\n \tfree_commit_list(list);\n \tfor (l = backup; l; l = l->next)\n \t\tclear_commit_marks(l->item, ONELINE_SEEN);\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex 4b8e5cc..bba2364 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -233,8 +233,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)\n \t\t\texpression = value;\n \t\tif (regcomp(&reg->re, expression, 0))\n \t\t\tdie(\"Invalid regexp to look for hunk header: %s\", expression);\n-\t\tif (buffer)\n-\t\t\tfree(buffer);\n+\t\tfree(buffer);\n \t\tvalue = ep + 1;\n \t}\n }\n--\n1.5.4.2.134.g82883\n"},{"id":"69596","messageId":"7vzlts9ag8.fsf@gitster.siamese.dyndns.org","threadId":"12162","inReplyTo":"87skznhqk6.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-22T17:18:15Z","receivedAt":"2008-02-22T17:18:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> ...\n> Here's an updated patch:\n> ...\n> Date: Thu, 31 Jan 2008 18:26:32 +0100\n> Subject: [PATCH] Avoid unnecessary \"if-before-free\" tests.\n>\n> This change removes all obvious useless if-before-free tests.\n\nThanks.  I'll queue this probably in 'next' for now, but we\nwould want a conditional workaround for git-compat-util.h before\nwe push it out to 'master'.\n"},{"id":"69597","messageId":"87ir0gx5bn.fsf@rho.meyering.net","threadId":"12162","inReplyTo":"7vzlts9ag8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-22T17:35:08Z","receivedAt":"2008-02-22T17:35:08Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Jim Meyering <jim@meyering.net> writes:\n>> ...\n>> Here's an updated patch:\n>> ...\n>> Date: Thu, 31 Jan 2008 18:26:32 +0100\n>> Subject: [PATCH] Avoid unnecessary \"if-before-free\" tests.\n>>\n>> This change removes all obvious useless if-before-free tests.\n>\n> Thanks.  I'll queue this probably in 'next' for now, but we\n> would want a conditional workaround for git-compat-util.h before\n> we push it out to 'master'.\n\nOk.\nWould you like an autoconf test to check for working free?\n"},{"id":"69598","messageId":"7vskzk99fd.fsf@gitster.siamese.dyndns.org","threadId":"12162","inReplyTo":"87ir0gx5bn.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-22T17:40:22Z","receivedAt":"2008-02-22T17:40:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n>> Thanks.  I'll queue this probably in 'next' for now, but we\n>> would want a conditional workaround for git-compat-util.h before\n>> we push it out to 'master'.\n>\n> Ok.\n> Would you like an autoconf test to check for working free?\n\nSure, but that could be left for later rounds.\n\nWe usually first add manual configuration option to Makefile\n(say, NO_FREE_NULL=Unfortunately) and then set that symbol via\nautoconf only when somebody really cares about.\n"},{"id":"69619","messageId":"87tzk0tzjz.fsf@rho.meyering.net","threadId":"12162","inReplyTo":"7vskzk99fd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-22T22:08:00Z","receivedAt":"2008-02-22T22:08:00Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n\n> Jim Meyering <jim@meyering.net> writes:\n>\n>>> Thanks.  I'll queue this probably in 'next' for now, but we\n>>> would want a conditional workaround for git-compat-util.h before\n>>> we push it out to 'master'.\n\nHere you go.\nThe only changed bits (other than rebase-induced) are the\nadditions to Makefile, git-compat-util.h, and the log text.\n\n>From b4ad1fc0325d0b9e2905f6fac1cda88be2a28ddf Mon Sep 17 00:00:00 2001\nFrom: Jim Meyering <meyering@redhat.com>\nDate: Thu, 31 Jan 2008 18:26:32 +0100\nSubject: [PATCH] Avoid unnecessary \"if-before-free\" tests.\n\nThis change removes all obvious useless if-before-free tests.\nE.g., it replaces code like this:\n\n        if (some_expression)\n                free (some_expression);\n\nwith the now-equivalent:\n\n        free (some_expression);\n\nIt is equivalent not just because POSIX has required free(NULL)\nto work for a long time, but simply because it has worked for\nso long that no reasonable porting target fails the test.\nHere's some evidence from nearly 1.5 years ago:\n\n    http://www.winehq.org/pipermail/wine-patches/2006-October/031544.html\n\nFYI, the change below was prepared by running the following:\n\n  git ls-files -z | xargs -0 \\\n  perl -0x3b -pi -e \\\n    's/\\bif\\s*\\(\\s*(\\S+?)(?:\\s*!=\\s*NULL)?\\s*\\)\\s+(free\\s*\\(\\s*\\1\\s*\\))/$2/s'\n\nNote however, that it doesn't handle brace-enclosed blocks like\n\"if (x) { free (x); }\".  But that's ok, since there were none like\nthat in git sources.\n\nBeware: if you do use the above snippet, note that it can\nproduce syntactically invalid C code.  That happens when the\naffected \"if\"-statement has a matching \"else\".\nE.g., it would transform this\n\n  if (x)\n    free (x);\n  else\n    foo ();\n\ninto this:\n\n  free (x);\n  else\n    foo ();\n\nThere were none of those here, either.\n\nIf you're interested in automating detection of the useless\ntests, you might like the useless-if-before-free script in gnulib:\n[it *does* detect brace-enclosed free statements, and has a --name=S\n option to make it detect free-like functions with different names]\n\n  http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=build-aux/useless-if-before-free\n\nAddendum:\n  Remove one more, slightly different, (in imap-send.c),\n    spotted by Jean-Luc Herren <jlh@gmx.ch>.\n  Provide a portability crutch, in case we ever find a system\n    on which free(NULL) misbehaves:\n    * git-compat-util.c (gitfree) [FREE_NULL_CRASHES]: Define function.\n    And #define free gitfree.\n    Suggested by Johannes Schindelin.\n    * Makefile [FREE_NULL_CRASHES]: Define via COMPAT_CFLAGS.\n\nSigned-off-by: Jim Meyering <meyering@redhat.com>\n---\n Makefile               |    7 +++++++\n builtin-blame.c        |    3 +--\n builtin-branch.c       |    9 +++------\n builtin-fast-export.c  |    3 +--\n builtin-http-fetch.c   |    3 +--\n builtin-pack-objects.c |    3 +--\n builtin-revert.c       |    3 +--\n connect.c              |    3 +--\n diff.c                 |    9 +++------\n dir.c                  |    3 +--\n git-compat-util.h      |    9 +++++++++\n http-push.c            |   18 ++++++------------\n imap-send.c            |    5 ++---\n interpolate.c          |    3 +--\n pretty.c               |    3 +--\n remote.c               |    3 +--\n setup.c                |    3 +--\n sha1_name.c            |    6 ++----\n xdiff-interface.c      |    3 +--\n 19 files changed, 44 insertions(+), 55 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d33a556..3caf5b8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -144,6 +144,9 @@ all::\n # is a simplified version of the merge sort used in glibc. This is\n # recommended if Git triggers O(n^2) behavior in your platform's qsort().\n #\n+# Define FREE_NULL_CRASHES if your are on a system (e.g., SunOS4)\n+# for which free(NULL) segfaults.\n+#\n\n GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -743,6 +746,10 @@ ifdef THREADED_DELTA_SEARCH\n \tEXTLIBS += -lpthread\n endif\n\n+ifdef FREE_NULL_CRASHES\n+\tCOMPAT_CFLAGS += -DFREE_NULL_CRASHES\n+endif\n+\n ifeq ($(TCLTK_PATH),)\n NO_TCLTK=NoThanks\n endif\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 59d7237..bfd562d 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -123,8 +123,7 @@ static inline struct origin *origin_incref(struct origin *o)\n static void origin_decref(struct origin *o)\n {\n \tif (o && --o->refcnt <= 0) {\n-\t\tif (o->file.ptr)\n-\t\t\tfree(o->file.ptr);\n+\t\tfree(o->file.ptr);\n \t\tfree(o);\n \t}\n }\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 9edf2eb..7917700 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -126,8 +126,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t\tcontinue;\n \t\t}\n\n-\t\tif (name)\n-\t\t\tfree(name);\n+\t\tfree(name);\n\n \t\tname = xstrdup(mkpath(fmt, argv[i]));\n \t\tif (!resolve_ref(name, sha1, 1, NULL)) {\n@@ -172,8 +171,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t}\n \t}\n\n-\tif (name)\n-\t\tfree(name);\n+\tfree(name);\n\n \treturn(ret);\n }\n@@ -490,8 +488,7 @@ static void create_branch(const char *name, const char *start_name,\n \tif (write_ref_sha1(lock, sha1, msg) < 0)\n \t\tdie(\"Failed to write ref: %s.\", strerror(errno));\n\n-\tif (real_ref)\n-\t\tfree(real_ref);\n+\tfree(real_ref);\n }\n\n static void rename_branch(const char *oldname, const char *newname, int force)\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex f741df5..49b54de 100755\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -196,8 +196,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n \t\t\t  ? strlen(reencoded) : message\n \t\t\t  ? strlen(message) : 0),\n \t       reencoded ? reencoded : message ? message : \"\");\n-\tif (reencoded)\n-\t\tfree(reencoded);\n+\tfree(reencoded);\n\n \tfor (i = 0, p = commit->parents; p; p = p->next) {\n \t\tint mark = get_object_mark(&p->item->object);\ndiff --git a/builtin-http-fetch.c b/builtin-http-fetch.c\nindex 7f450c6..299093f 100644\n--- a/builtin-http-fetch.c\n+++ b/builtin-http-fetch.c\n@@ -80,8 +80,7 @@ int cmd_http_fetch(int argc, const char **argv, const char *prefix)\n\n \twalker_free(walker);\n\n-\tif (rewritten_url)\n-\t\tfree(rewritten_url);\n+\tfree(rewritten_url);\n\n \treturn rc;\n }\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex d2bb12e..7dff653 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1428,8 +1428,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t * accounting lock.  Compiler will optimize the strangeness\n \t * away when THREADED_DELTA_SEARCH is not defined.\n \t */\n-\tif (trg_entry->delta_data)\n-\t\tfree(trg_entry->delta_data);\n+\tfree(trg_entry->delta_data);\n \tcache_lock();\n \tif (trg_entry->delta_data) {\n \t\tdelta_cache_size -= trg_entry->delta_size;\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex e219859..b6dee6a 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -397,8 +397,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\telse\n \t\t\treturn execl_git_cmd(\"commit\", \"-n\", \"-F\", defmsg, NULL);\n \t}\n-\tif (reencoded_message)\n-\t\tfree(reencoded_message);\n+\tfree(reencoded_message);\n\n \treturn 0;\n }\ndiff --git a/connect.c b/connect.c\nindex 5ac3572..d12b105 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -68,8 +68,7 @@ struct ref **get_remote_heads(int in, struct ref **list,\n\n \t\tname_len = strlen(name);\n \t\tif (len != name_len + 41) {\n-\t\t\tif (server_capabilities)\n-\t\t\t\tfree(server_capabilities);\n+\t\t\tfree(server_capabilities);\n \t\t\tserver_capabilities = xstrdup(name + name_len + 1);\n \t\t}\n\ndiff --git a/diff.c b/diff.c\nindex c30c252..68a1a99 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -118,8 +118,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v\n \t\tpp->next = funcname_pattern_list;\n \t\tfuncname_pattern_list = pp;\n \t}\n-\tif (pp->pattern)\n-\t\tfree(pp->pattern);\n+\tfree(pp->pattern);\n \tpp->pattern = xstrdup(value);\n \treturn 0;\n }\n@@ -492,10 +491,8 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \t\t\t\tecbdata->diff_words->plus.text.size)\n \t\t\tdiff_words_show(ecbdata->diff_words);\n\n-\t\tif (ecbdata->diff_words->minus.text.ptr)\n-\t\t\tfree (ecbdata->diff_words->minus.text.ptr);\n-\t\tif (ecbdata->diff_words->plus.text.ptr)\n-\t\t\tfree (ecbdata->diff_words->plus.text.ptr);\n+\t\tfree (ecbdata->diff_words->minus.text.ptr);\n+\t\tfree (ecbdata->diff_words->plus.text.ptr);\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\ndiff --git a/dir.c b/dir.c\nindex 1f507da..edc458e 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -704,8 +704,7 @@ static struct path_simplify *create_simplify(const char **pathspec)\n\n static void free_simplify(struct path_simplify *simplify)\n {\n-\tif (simplify)\n-\t\tfree(simplify);\n+\tfree(simplify);\n }\n\n int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2a40703..911fdec 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -225,6 +225,15 @@ static inline char *gitstrchrnul(const char *s, int c)\n }\n #endif\n\n+#ifdef FREE_NULL_CRASHES\n+inline void gitfree(void *ptr)\n+{\n+\tif (ptr)\n+\t\tfree(ptr);\n+}\n+#define free gitfree\n+#endif\n+\n extern void release_pack_memory(size_t, int);\n\n static inline char* xstrdup(const char *str)\ndiff --git a/http-push.c b/http-push.c\nindex 0beb740..406270f 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -664,8 +664,7 @@ static void release_request(struct transfer_request *request)\n \t\tclose(request->local_fileno);\n \tif (request->local_stream)\n \t\tfclose(request->local_stream);\n-\tif (request->url != NULL)\n-\t\tfree(request->url);\n+\tfree(request->url);\n \tfree(request);\n }\n\n@@ -1283,10 +1282,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \tstrbuf_release(&in_buffer);\n\n \tif (lock->token == NULL || lock->timeout <= 0) {\n-\t\tif (lock->token != NULL)\n-\t\t\tfree(lock->token);\n-\t\tif (lock->owner != NULL)\n-\t\t\tfree(lock->owner);\n+\t\tfree(lock->token);\n+\t\tfree(lock->owner);\n \t\tfree(url);\n \t\tfree(lock);\n \t\tlock = NULL;\n@@ -1344,8 +1341,7 @@ static int unlock_remote(struct remote_lock *lock)\n \t\t\tprev->next = prev->next->next;\n \t}\n\n-\tif (lock->owner != NULL)\n-\t\tfree(lock->owner);\n+\tfree(lock->owner);\n \tfree(lock->url);\n \tfree(lock->token);\n \tfree(lock);\n@@ -2035,8 +2031,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)\n \t}\n \tfree(url);\n\n-\tif (*symref != NULL)\n-\t\tfree(*symref);\n+\tfree(*symref);\n \t*symref = NULL;\n \thashclr(sha1);\n\n@@ -2435,8 +2430,7 @@ int main(int argc, char **argv)\n \t}\n\n  cleanup:\n-\tif (rewritten_url)\n-\t\tfree(rewritten_url);\n+\tfree(rewritten_url);\n \tif (info_ref_lock)\n \t\tunlock_remote(info_ref_lock);\n \tfree(remote);\ndiff --git a/imap-send.c b/imap-send.c\nindex 9025d9a..10cce15 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -472,7 +472,7 @@ v_issue_imap_cmd( imap_store_t *ctx, struct imap_cmd_cb *cb,\n \tif (socket_write( &imap->buf.sock, buf, bufl ) != bufl) {\n \t\tfree( cmd->cmd );\n \t\tfree( cmd );\n-\t\tif (cb && cb->data)\n+\t\tif (cb)\n \t\t\tfree( cb->data );\n \t\treturn NULL;\n \t}\n@@ -858,8 +858,7 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )\n \t\t  normal:\n \t\t\tif (cmdp->cb.done)\n \t\t\t\tcmdp->cb.done( ctx, cmdp, resp );\n-\t\t\tif (cmdp->cb.data)\n-\t\t\t\tfree( cmdp->cb.data );\n+\t\t\tfree( cmdp->cb.data );\n \t\t\tfree( cmdp->cmd );\n \t\t\tfree( cmdp );\n \t\t\tif (!tcmd || tcmd == cmdp)\ndiff --git a/interpolate.c b/interpolate.c\nindex 6ef53f2..7f03bd9 100644\n--- a/interpolate.c\n+++ b/interpolate.c\n@@ -11,8 +11,7 @@ void interp_set_entry(struct interp *table, int slot, const char *value)\n \tchar *oldval = table[slot].value;\n \tchar *newval = NULL;\n\n-\tif (oldval)\n-\t\tfree(oldval);\n+\tfree(oldval);\n\n \tif (value)\n \t\tnewval = xstrdup(value);\ndiff --git a/pretty.c b/pretty.c\nindex 997f583..4bf3be2 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -30,8 +30,7 @@ enum cmit_fmt get_commit_format(const char *arg)\n \tif (*arg == '=')\n \t\targ++;\n \tif (!prefixcmp(arg, \"format:\")) {\n-\t\tif (user_format)\n-\t\t\tfree(user_format);\n+\t\tfree(user_format);\n \t\tuser_format = xstrdup(arg + 7);\n \t\treturn CMIT_FMT_USERFORMAT;\n \t}\ndiff --git a/remote.c b/remote.c\nindex 6b56473..ae1ef57 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -506,8 +506,7 @@ void free_refs(struct ref *ref)\n \tstruct ref *next;\n \twhile (ref) {\n \t\tnext = ref->next;\n-\t\tif (ref->peer_ref)\n-\t\t\tfree(ref->peer_ref);\n+\t\tfree(ref->peer_ref);\n \t\tfree(ref);\n \t\tref = next;\n \t}\ndiff --git a/setup.c b/setup.c\nindex bc80301..6c96ae8 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -448,8 +448,7 @@ int check_repository_format_version(const char *var, const char *value)\n \t} else if (strcmp(var, \"core.worktree\") == 0) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tif (git_work_tree_cfg)\n-\t\t\tfree(git_work_tree_cfg);\n+\t\tfree(git_work_tree_cfg);\n \t\tgit_work_tree_cfg = xstrdup(value);\n \t\tinside_work_tree = -1;\n \t}\ndiff --git a/sha1_name.c b/sha1_name.c\nindex c2805e7..9d088cc 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -625,8 +625,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n \t\tcommit = pop_most_recent_commit(&list, ONELINE_SEEN);\n \t\tif (!parse_object(commit->object.sha1))\n \t\t\tcontinue;\n-\t\tif (temp_commit_buffer)\n-\t\t\tfree(temp_commit_buffer);\n+\t\tfree(temp_commit_buffer);\n \t\tif (commit->buffer)\n \t\t\tp = commit->buffer;\n \t\telse {\n@@ -643,8 +642,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)\n \t\t\tbreak;\n \t\t}\n \t}\n-\tif (temp_commit_buffer)\n-\t\tfree(temp_commit_buffer);\n+\tfree(temp_commit_buffer);\n \tfree_commit_list(list);\n \tfor (l = backup; l; l = l->next)\n \t\tclear_commit_marks(l->item, ONELINE_SEEN);\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex 4b8e5cc..bba2364 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -233,8 +233,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)\n \t\t\texpression = value;\n \t\tif (regcomp(&reg->re, expression, 0))\n \t\t\tdie(\"Invalid regexp to look for hunk header: %s\", expression);\n-\t\tif (buffer)\n-\t\t\tfree(buffer);\n+\t\tfree(buffer);\n \t\tvalue = ep + 1;\n \t}\n }\n--\n1.5.4.2.185.gf5f8\n"},{"id":"69625","messageId":"7vd4qo7fsc.fsf@gitster.siamese.dyndns.org","threadId":"12162","inReplyTo":"87tzk0tzjz.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-22T23:05:55Z","receivedAt":"2008-02-22T23:05:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> This change removes all obvious useless if-before-free tests.\n> E.g., it replaces code like this:\n>\n>         if (some_expression)\n>                 free (some_expression);\n>\n> with the now-equivalent:\n>\n>         free (some_expression);\n>\n> ...\n>\n> If you're interested in automating detection of the useless\n> tests, you might like the useless-if-before-free script in gnulib:\n> [it *does* detect brace-enclosed free statements, and has a --name=S\n>  option to make it detect free-like functions with different names]\n\nWhile I have your attention ;-)\n\nI am not interested in automating useless \"if (x) free(x)\"\ntests, but one thing I recently wanted but did not know a handy\ntool for was to find all the calls to free() that free a pointer\nto an object of a particular type.  More specifically, we seem\nto allocate and free many \"struct commit_list\", and I wanted to\nintroduce a custom bulk allocator.  Allocate many of them in a\nblock, hand out one by one, and tell callers to hand them back\nnot to free() but to the allocator so that it can keep the\nreturned ones on a linked list and hand them back again when the\nnext call wanted to allocate one without actually calling\nxmalloc()).  But in order to do so, missed conversion from\nmalloc() to the custom allocator is not fatal (just wasteful),\nbut forgetting to convert free() really is.\n\nI guess sparse could be hacked to do that, but do GNU folks have\nsome checker like that?\n"},{"id":"69680","messageId":"118833cc0802230530l104acc72k20ceb4b5adcff937@mail.gmail.com","threadId":"12162","inReplyTo":"87tzk0tzjz.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2008-02-23T13:30:42Z","receivedAt":"2008-02-23T13:30:42Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"> +inline void gitfree(void *ptr)\n>  +{\n>  +       if (ptr)\n>  +               free(ptr);\n>  +}\n>  +#define free gitfree\n>  +#endif\n\nI am wondering why you do it this way.  \"#define free gitfree\" is just\nnot valid in a C program that includes the relevant standard header.\n\"free\" is a reserved symbol.\n\nTo stay within the standard, do the define the other way and use\ngitfree everywhere.\n\nMorten\n"},{"id":"69775","messageId":"alpine.DEB.1.00.0802241100350.6881@eeepc-johanness","threadId":"12162","inReplyTo":"118833cc0802230530l104acc72k20ceb4b5adcff937@mail.gmail.com","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-24T10:06:07Z","receivedAt":"2008-02-24T10:06:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 23 Feb 2008, Morten Welinder wrote:\n\n> > +inline void gitfree(void *ptr)\n> >  +{\n> >  +       if (ptr)\n> >  +               free(ptr);\n> >  +}\n> >  +#define free gitfree\n> >  +#endif\n> \n> I am wondering why you do it this way.  \"#define free gitfree\" is just\n> not valid in a C program that includes the relevant standard header.\n> \"free\" is a reserved symbol.\n> \n> To stay within the standard, do the define the other way and use\n> gitfree everywhere.\n\nWe do it this way for other things like fopen, too.\n\nBesides, I think that there should be at least one _real_ case where it \nactually _breaks_ before we have a big, ugly, change where it is easy to \noverlook a non-converted \"free()\", instead of a nice, clean and short \npatch.\n\nCiao,\nDscho\n"},{"id":"69807","messageId":"877igup6fl.fsf@rho.meyering.net","threadId":"12162","inReplyTo":"7vd4qo7fsc.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-02-24T18:15:10Z","receivedAt":"2008-02-24T18:15:10Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n...\n>> If you're interested in automating detection of the useless\n>> tests, you might like the useless-if-before-free script in gnulib:\n>> [it *does* detect brace-enclosed free statements, and has a --name=S\n>>  option to make it detect free-like functions with different names]\n>\n> While I have your attention ;-)\n\nHi Jun,\n\nNo excuse required ;-)\n\n> I am not interested in automating useless \"if (x) free(x)\" tests,\n\nYeah, that particular one is not a big deal, but whenever I take\nthe time to make a sweeping change, I find it's worth a little\nmore to automate a check to preserve the goal state.\n\n> but one thing I recently wanted but did not know a handy\n> tool for was to find all the calls to free() that free a pointer\n> to an object of a particular type.\n...\n> I guess sparse could be hacked to do that, but do GNU folks have\n> some checker like that?\n\nA general purpose tool to do something like that would be very useful.\nI thought of cscope and eclipse, but as far as I know,\nneither of them can perform such a query.\n\nThis made me think of the dwarves package/paper:\n\n    7 dwarves\n    https://ols2006.108.redhat.com/2007/Reprints/melo-Reprint.pdf\n\nThis looked promising at first, but it annotates function _definitions_\nfor run-time data collection, while you want to look at uses, which\ncan be done statically:\n\n    3.4   ctracer\n\n    A class tracer, ctracer is an experiment in creating valid source\n    code from the DWARF information.  For ctracer a method is any\n    function that receives as one of its parameters a pointer to a\n    specified struct. It looks for all such methods and generates\n    kprobes entry and exit functions. At these probe points it\n    collects information about the data structure internal state,\n    saving the values in its members in that point in time, and\n    records it in a relay buffer. The data is later collected in\n    userspace and post-processed, generating html + CSS callgraphs.\n\nToo bad coverity is closed-source.  I'll bet it could do this easily.\n\nMaybe hacking sparse is the way to go, after all.\n"},{"id":"69962","messageId":"20080226065953.GA25073@informatik.uni-freiburg.de","threadId":"12162","inReplyTo":"7vd4qo7fsc.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Uwe Kleine-König","fromEmail":"ukleinek@informatik.uni-freiburg.de","sentAt":"2008-02-26T06:59:53Z","receivedAt":"2008-02-26T06:59:53Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Junio,\n\n> I am not interested in automating useless \"if (x) free(x)\"\n> tests, but one thing I recently wanted but did not know a handy\n> tool for was to find all the calls to free() that free a pointer\n> to an object of a particular type.  More specifically, we seem\n> to allocate and free many \"struct commit_list\", and I wanted to\n> introduce a custom bulk allocator.  Allocate many of them in a\n> block, hand out one by one, and tell callers to hand them back\n> not to free() but to the allocator so that it can keep the\n> returned ones on a linked list and hand them back again when the\n> next call wanted to allocate one without actually calling\n> xmalloc()).  But in order to do so, missed conversion from\n> malloc() to the custom allocator is not fatal (just wasteful),\n> but forgetting to convert free() really is.\nMaybe http://www.emn.fr/x-info/coccinelle/ can help you?\n\nI think it could automate if (x) free(x), too.\n\nBest regards\nUwe\n\n-- \nUwe Kleine-König\n"},{"id":"106325","messageId":"e2b179460902260548g32e5be97p138a9d8fb5d5ef78@mail.gmail.com","threadId":"12162","inReplyTo":"877igup6fl.fsf@rho.meyering.net","subject":"Re: [PATCH] Remove useless if-before-free tests.","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2009-02-26T13:48:25Z","receivedAt":"2009-02-26T13:48:25Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"2008/2/24 Jim Meyering <jim@meyering.net>:\n> Too bad coverity is closed-source.  I'll bet it could do this easily.\n\nhttp://scan.coverity.com/\n\nCourtesy of announcement on LWN.\n\ngit currently isn't on the ladder, but could be submitted by our\ngentle leader, if the license terms[1] were acceptable.\n\nMike\n\n[1] http://scan.coverity.com/policy.html#license\n"}]}