{"thread":{"id":"39245","subject":"[PATCH] prefix_path(): Unconditionally free result of prefix_path","startedAt":"2015-05-04T19:11:54Z","lastAt":"2015-05-05T18:09:11Z","messageCount":8,"participants":["Stefan Beller","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"260538","messageId":"1430766714-22368-1-git-send-email-sbeller@google.com","threadId":"39245","inReplyTo":null,"subject":"[PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-04T19:11:54Z","receivedAt":"2015-05-04T19:11:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"prefix_path() always returns a newly allocated string since\nd089eba (setup: sanitize absolute and funny paths in get_pathspec(),\n2008-01-28)\n\nAdditionally the const is dropped from the pointers, so the call to\nfree doesn't need a cast.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nNotes:\n    Thanks for all the suggestions!\n    They are incorporated into this version of the patch.\n\n builtin/checkout-index.c | 10 ++++------\n builtin/update-index.c   |  5 ++---\n 2 files changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex 9ca2da1..8028c37 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -241,7 +241,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t/* Check out named files first */\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-\t\tconst char *p;\n+\t\tchar *p;\n \n \t\tif (all)\n \t\t\tdie(\"git checkout-index: don't mix '--all' and explicit filenames\");\n@@ -249,8 +249,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"git checkout-index: don't mix '--stdin' and explicit filenames\");\n \t\tp = prefix_path(prefix, prefix_length, arg);\n \t\tcheckout_file(p, prefix);\n-\t\tif (p < arg || p > arg + strlen(arg))\n-\t\t\tfree((char *)p);\n+\t\tfree(p);\n \t}\n \n \tif (read_from_stdin) {\n@@ -260,7 +259,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"git checkout-index: don't mix '--all' and '--stdin'\");\n \n \t\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n-\t\t\tconst char *p;\n+\t\t\tchar *p;\n \t\t\tif (line_termination && buf.buf[0] == '\"') {\n \t\t\t\tstrbuf_reset(&nbuf);\n \t\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n@@ -269,8 +268,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tp = prefix_path(prefix, prefix_length, buf.buf);\n \t\t\tcheckout_file(p, prefix);\n-\t\t\tif (p < buf.buf || p > buf.buf + buf.len)\n-\t\t\t\tfree((char *)p);\n+\t\t\tfree(p);\n \t\t}\n \t\tstrbuf_release(&nbuf);\n \t\tstrbuf_release(&buf);\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 6271b54..a92eed2 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -532,10 +532,9 @@ static int do_unresolve(int ac, const char **av,\n \n \tfor (i = 1; i < ac; i++) {\n \t\tconst char *arg = av[i];\n-\t\tconst char *p = prefix_path(prefix, prefix_length, arg);\n+\t\tchar *p = prefix_path(prefix, prefix_length, arg);\n \t\terr |= unresolve_one(p);\n-\t\tif (p < arg || p > arg + strlen(arg))\n-\t\t\tfree((char *)p);\n+\t\tfree(p);\n \t}\n \treturn err;\n }\n-- \n2.4.0.rc3.16.g0ab00b9\n"},{"id":"260543","messageId":"CAPig+cSuCouNCuKa99mct4UMPykuMVy3+7sqB6y+v+UtP2oeTw@mail.gmail.com","threadId":"39245","inReplyTo":"1430766714-22368-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-04T20:19:26Z","receivedAt":"2015-05-04T20:19:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 4, 2015 at 3:11 PM, Stefan Beller <sbeller@google.com> wrote:\n> prefix_path(): Unconditionally free result of prefix_path\n\nSlightly redundant mention of \"prefix_path\". Also, prevailing custom\nis to drop capitalization.\n\n> prefix_path() always returns a newly allocated string since\n> d089eba (setup: sanitize absolute and funny paths in get_pathspec(),\n> 2008-01-28)\n\nI'd probably turn this sentence fragment into a proper sentence:\n\n    As of d089eba (...), prefix_path() always returns a newly\n    allocated string, so free its result unconditionally.\n\n> Additionally the const is dropped from the pointers, so the call to\n> free doesn't need a cast.\n\nImperative mood:\n\n    Additionally, drop the const from variables to which the\n    prefix_path() result is assigned so they can be free()'d\n    without having to cast-away constness.\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n> Notes:\n>     Thanks for all the suggestions!\n>     They are incorporated into this version of the patch.\n\nThanks, this version looks much better.\n\nFWIW, with or without addressing the very minor nits above:\n\nReviewed-by: Eric Sunshine <sunshine@sunshineco.com>\n\n> diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\n> index 9ca2da1..8028c37 100644\n> --- a/builtin/checkout-index.c\n> +++ b/builtin/checkout-index.c\n> @@ -241,7 +241,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n>         /* Check out named files first */\n>         for (i = 0; i < argc; i++) {\n>                 const char *arg = argv[i];\n> -               const char *p;\n> +               char *p;\n>\n>                 if (all)\n>                         die(\"git checkout-index: don't mix '--all' and explicit filenames\");\n> @@ -249,8 +249,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n>                         die(\"git checkout-index: don't mix '--stdin' and explicit filenames\");\n>                 p = prefix_path(prefix, prefix_length, arg);\n>                 checkout_file(p, prefix);\n> -               if (p < arg || p > arg + strlen(arg))\n> -                       free((char *)p);\n> +               free(p);\n>         }\n>\n>         if (read_from_stdin) {\n> @@ -260,7 +259,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n>                         die(\"git checkout-index: don't mix '--all' and '--stdin'\");\n>\n>                 while (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n> -                       const char *p;\n> +                       char *p;\n>                         if (line_termination && buf.buf[0] == '\"') {\n>                                 strbuf_reset(&nbuf);\n>                                 if (unquote_c_style(&nbuf, buf.buf, NULL))\n> @@ -269,8 +268,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n>                         }\n>                         p = prefix_path(prefix, prefix_length, buf.buf);\n>                         checkout_file(p, prefix);\n> -                       if (p < buf.buf || p > buf.buf + buf.len)\n> -                               free((char *)p);\n> +                       free(p);\n>                 }\n>                 strbuf_release(&nbuf);\n>                 strbuf_release(&buf);\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index 6271b54..a92eed2 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -532,10 +532,9 @@ static int do_unresolve(int ac, const char **av,\n>\n>         for (i = 1; i < ac; i++) {\n>                 const char *arg = av[i];\n> -               const char *p = prefix_path(prefix, prefix_length, arg);\n> +               char *p = prefix_path(prefix, prefix_length, arg);\n>                 err |= unresolve_one(p);\n> -               if (p < arg || p > arg + strlen(arg))\n> -                       free((char *)p);\n> +               free(p);\n>         }\n>         return err;\n>  }\n> --\n> 2.4.0.rc3.16.g0ab00b9\n"},{"id":"260564","messageId":"xmqq8ud4ou2b.fsf@gitster.dls.corp.google.com","threadId":"39245","inReplyTo":"CAPig+cSuCouNCuKa99mct4UMPykuMVy3+7sqB6y+v+UtP2oeTw@mail.gmail.com","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-04T22:29:32Z","receivedAt":"2015-05-04T22:29:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Mon, May 4, 2015 at 3:11 PM, Stefan Beller <sbeller@google.com> wrote:\n>> prefix_path(): Unconditionally free result of prefix_path\n>\n> Slightly redundant mention of \"prefix_path\". Also, prevailing custom\n> is to drop capitalization.\n>\n>> prefix_path() always returns a newly allocated string since\n>> d089eba (setup: sanitize absolute and funny paths in get_pathspec(),\n>> 2008-01-28)\n>\n> I'd probably turn this sentence fragment into a proper sentence:\n>\n>     As of d089eba (...), prefix_path() always returns a newly\n>     allocated string, so free its result unconditionally.\n>\n>> Additionally the const is dropped from the pointers, so the call to\n>> free doesn't need a cast.\n>\n> Imperative mood:\n>\n>     Additionally, drop the const from variables to which the\n>     prefix_path() result is assigned so they can be free()'d\n>     without having to cast-away constness.\n>\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>> ---\n>>\n>> Notes:\n>>     Thanks for all the suggestions!\n>>     They are incorporated into this version of the patch.\n>\n> Thanks, this version looks much better.\n>\n> FWIW, with or without addressing the very minor nits above:\n>\n> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>\n\nThanks, both.\n\nTo save a round-trip, I'll munge the log message myself stealing\nEric's suggestions.\n"},{"id":"260579","messageId":"20150505032158.GA23587@peff.net","threadId":"39245","inReplyTo":"1430766714-22368-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-05-05T03:21:59Z","receivedAt":"2015-05-05T03:21:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 04, 2015 at 12:11:54PM -0700, Stefan Beller wrote:\n\n> prefix_path() always returns a newly allocated string since\n> d089eba (setup: sanitize absolute and funny paths in get_pathspec(),\n> 2008-01-28)\n> \n> Additionally the const is dropped from the pointers, so the call to\n> free doesn't need a cast.\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n> \n> Notes:\n>     Thanks for all the suggestions!\n>     They are incorporated into this version of the patch.\n> \n>  builtin/checkout-index.c | 10 ++++------\n>  builtin/update-index.c   |  5 ++---\n>  2 files changed, 6 insertions(+), 9 deletions(-)\n\nShould we also squash in these sites? I think they are adequately\ncovered under the proposed log message.\n\nFound by grepping for prefix_path calls. The only remainders are:\n\n  1. in blame, we assign the result to a const char that may also point\n     straight into to argv, but we never actually free either way\n\n  2. test-path-utils does not free at all, but we probably don't care\n     either way\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex a92eed2..0665b31 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -870,14 +870,14 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tcase PARSE_OPT_DONE:\n \t\t{\n \t\t\tconst char *path = ctx.argv[0];\n-\t\t\tconst char *p;\n+\t\t\tchar *p;\n \n \t\t\tsetup_work_tree();\n \t\t\tp = prefix_path(prefix, prefix_length, path);\n \t\t\tupdate_one(p);\n \t\t\tif (set_executable_bit)\n \t\t\t\tchmod_path(set_executable_bit, p);\n-\t\t\tfree((char *)p);\n+\t\t\tfree(p);\n \t\t\tctx.argc--;\n \t\t\tctx.argv++;\n \t\t\tbreak;\n@@ -908,7 +908,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \n \t\tsetup_work_tree();\n \t\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n-\t\t\tconst char *p;\n+\t\t\tchar *p;\n \t\t\tif (line_termination && buf.buf[0] == '\"') {\n \t\t\t\tstrbuf_reset(&nbuf);\n \t\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n@@ -919,7 +919,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tupdate_one(p);\n \t\t\tif (set_executable_bit)\n \t\t\t\tchmod_path(set_executable_bit, p);\n-\t\t\tfree((char *)p);\n+\t\t\tfree(p);\n \t\t}\n \t\tstrbuf_release(&nbuf);\n \t\tstrbuf_release(&buf);\n"},{"id":"260594","messageId":"CAGZ79kZjeG8UG5ALE-KSO52fD5gJk4xks=VtSV9bHQVA=ST+5Q@mail.gmail.com","threadId":"39245","inReplyTo":"20150505032158.GA23587@peff.net","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-05T16:28:40Z","receivedAt":"2015-05-05T16:28:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, May 4, 2015 at 8:21 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, May 04, 2015 at 12:11:54PM -0700, Stefan Beller wrote:\n>\n>> prefix_path() always returns a newly allocated string since\n>> d089eba (setup: sanitize absolute and funny paths in get_pathspec(),\n>> 2008-01-28)\n>>\n>> Additionally the const is dropped from the pointers, so the call to\n>> free doesn't need a cast.\n>>\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>> ---\n>>\n>> Notes:\n>>     Thanks for all the suggestions!\n>>     They are incorporated into this version of the patch.\n>>\n>>  builtin/checkout-index.c | 10 ++++------\n>>  builtin/update-index.c   |  5 ++---\n>>  2 files changed, 6 insertions(+), 9 deletions(-)\n>\n> Should we also squash in these sites? I think they are adequately\n> covered under the proposed log message.\n\nThat sounds good to me.\n\n>\n> Found by grepping for prefix_path calls. The only remainders are:\n>\n>   1. in blame, we assign the result to a const char that may also point\n>      straight into to argv, but we never actually free either way\n>\n>   2. test-path-utils does not free at all, but we probably don't care\n>      either way\n>\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index a92eed2..0665b31 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -870,14 +870,14 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n>                 case PARSE_OPT_DONE:\n>                 {\n>                         const char *path = ctx.argv[0];\n> -                       const char *p;\n> +                       char *p;\n>\n>                         setup_work_tree();\n>                         p = prefix_path(prefix, prefix_length, path);\n>                         update_one(p);\n>                         if (set_executable_bit)\n>                                 chmod_path(set_executable_bit, p);\n> -                       free((char *)p);\n> +                       free(p);\n>                         ctx.argc--;\n>                         ctx.argv++;\n>                         break;\n> @@ -908,7 +908,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n>\n>                 setup_work_tree();\n>                 while (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n> -                       const char *p;\n> +                       char *p;\n>                         if (line_termination && buf.buf[0] == '\"') {\n>                                 strbuf_reset(&nbuf);\n>                                 if (unquote_c_style(&nbuf, buf.buf, NULL))\n> @@ -919,7 +919,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n>                         update_one(p);\n>                         if (set_executable_bit)\n>                                 chmod_path(set_executable_bit, p);\n> -                       free((char *)p);\n> +                       free(p);\n>                 }\n>                 strbuf_release(&nbuf);\n>                 strbuf_release(&buf);\n"},{"id":"260598","messageId":"xmqqr3qvdizd.fsf@gitster.dls.corp.google.com","threadId":"39245","inReplyTo":"CAGZ79kZjeG8UG5ALE-KSO52fD5gJk4xks=VtSV9bHQVA=ST+5Q@mail.gmail.com","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-05T17:36:38Z","receivedAt":"2015-05-05T17:36:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>> Should we also squash in these sites? I think they are adequately\n>> covered under the proposed log message.\n>\n> That sounds good to me.\n>>\n>> Found by grepping for prefix_path calls. The only remainders are:\n>>\n>>   1. in blame, we assign the result to a const char that may also point\n>>      straight into to argv, but we never actually free either way\n\nThe return value from add_prefix() that is what prefix_path()\nreturned eventually becomes scoreboard.path that needs to be kept\nduring the lifetime of the process, and I think there isn't much\npoint doing the \"free() immediately before exiting\".\n\n>>   2. test-path-utils does not free at all, but we probably don't care\n>>      either way\n\nAnyway, here is what I'd queue for now.\n\n-- >8 --\nFrom: Stefan Beller <sbeller@google.com>\nDate: Mon, 4 May 2015 12:11:54 -0700\nSubject: [PATCH] prefix_path(): unconditionally free results in the callers\n\nAs of d089ebaa (setup: sanitize absolute and funny paths in\nget_pathspec(), 2008-01-28), prefix_path() always returns a\nnewly allocated string, so callers should free its result.\n\nAdditionally, drop the const from variables to which the result of\nthe prefix_path() is assigned, so they can be free()'d without\nhaving to cast-away the constness.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nReviewed-by: Eric Sunshine <sunshine@sunshineco.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/checkout-index.c | 10 ++++------\n builtin/update-index.c   | 13 ++++++-------\n 2 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex 9ca2da1..8028c37 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -241,7 +241,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t/* Check out named files first */\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-\t\tconst char *p;\n+\t\tchar *p;\n \n \t\tif (all)\n \t\t\tdie(\"git checkout-index: don't mix '--all' and explicit filenames\");\n@@ -249,8 +249,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"git checkout-index: don't mix '--stdin' and explicit filenames\");\n \t\tp = prefix_path(prefix, prefix_length, arg);\n \t\tcheckout_file(p, prefix);\n-\t\tif (p < arg || p > arg + strlen(arg))\n-\t\t\tfree((char *)p);\n+\t\tfree(p);\n \t}\n \n \tif (read_from_stdin) {\n@@ -260,7 +259,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"git checkout-index: don't mix '--all' and '--stdin'\");\n \n \t\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n-\t\t\tconst char *p;\n+\t\t\tchar *p;\n \t\t\tif (line_termination && buf.buf[0] == '\"') {\n \t\t\t\tstrbuf_reset(&nbuf);\n \t\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n@@ -269,8 +268,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tp = prefix_path(prefix, prefix_length, buf.buf);\n \t\t\tcheckout_file(p, prefix);\n-\t\t\tif (p < buf.buf || p > buf.buf + buf.len)\n-\t\t\t\tfree((char *)p);\n+\t\t\tfree(p);\n \t\t}\n \t\tstrbuf_release(&nbuf);\n \t\tstrbuf_release(&buf);\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 6271b54..0665b31 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -532,10 +532,9 @@ static int do_unresolve(int ac, const char **av,\n \n \tfor (i = 1; i < ac; i++) {\n \t\tconst char *arg = av[i];\n-\t\tconst char *p = prefix_path(prefix, prefix_length, arg);\n+\t\tchar *p = prefix_path(prefix, prefix_length, arg);\n \t\terr |= unresolve_one(p);\n-\t\tif (p < arg || p > arg + strlen(arg))\n-\t\t\tfree((char *)p);\n+\t\tfree(p);\n \t}\n \treturn err;\n }\n@@ -871,14 +870,14 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tcase PARSE_OPT_DONE:\n \t\t{\n \t\t\tconst char *path = ctx.argv[0];\n-\t\t\tconst char *p;\n+\t\t\tchar *p;\n \n \t\t\tsetup_work_tree();\n \t\t\tp = prefix_path(prefix, prefix_length, path);\n \t\t\tupdate_one(p);\n \t\t\tif (set_executable_bit)\n \t\t\t\tchmod_path(set_executable_bit, p);\n-\t\t\tfree((char *)p);\n+\t\t\tfree(p);\n \t\t\tctx.argc--;\n \t\t\tctx.argv++;\n \t\t\tbreak;\n@@ -909,7 +908,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \n \t\tsetup_work_tree();\n \t\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n-\t\t\tconst char *p;\n+\t\t\tchar *p;\n \t\t\tif (line_termination && buf.buf[0] == '\"') {\n \t\t\t\tstrbuf_reset(&nbuf);\n \t\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n@@ -920,7 +919,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tupdate_one(p);\n \t\t\tif (set_executable_bit)\n \t\t\t\tchmod_path(set_executable_bit, p);\n-\t\t\tfree((char *)p);\n+\t\t\tfree(p);\n \t\t}\n \t\tstrbuf_release(&nbuf);\n \t\tstrbuf_release(&buf);\n-- \n2.4.0-311-gf1d9b8d\n"},{"id":"260606","messageId":"20150505175612.GA9709@peff.net","threadId":"39245","inReplyTo":"xmqqr3qvdizd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-05-05T17:56:12Z","receivedAt":"2015-05-05T17:56:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 05, 2015 at 10:36:38AM -0700, Junio C Hamano wrote:\n\n> Stefan Beller <sbeller@google.com> writes:\n> \n> >> Should we also squash in these sites? I think they are adequately\n> >> covered under the proposed log message.\n> >\n> > That sounds good to me.\n> >>\n> >> Found by grepping for prefix_path calls. The only remainders are:\n> >>\n> >>   1. in blame, we assign the result to a const char that may also point\n> >>      straight into to argv, but we never actually free either way\n> \n> The return value from add_prefix() that is what prefix_path()\n> returned eventually becomes scoreboard.path that needs to be kept\n> during the lifetime of the process, and I think there isn't much\n> point doing the \"free() immediately before exiting\".\n\nYeah, sorry, I meant to say that more explicitly, but clearly didn't. I\nthink it is fine as-is.\n\n> Anyway, here is what I'd queue for now.\n\nLooks good, thanks.\n\n-Peff\n"},{"id":"260608","messageId":"xmqq8ud2ew1k.fsf@gitster.dls.corp.google.com","threadId":"39245","inReplyTo":"20150505175612.GA9709@peff.net","subject":"Re: [PATCH] prefix_path(): Unconditionally free result of prefix_path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-05T18:09:11Z","receivedAt":"2015-05-05T18:09:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> >>   1. in blame, we assign the result to a const char that may also point\n>> >>      straight into to argv, but we never actually free either way\n>> \n>> The return value from add_prefix() that is what prefix_path()\n>> returned eventually becomes scoreboard.path that needs to be kept\n>> during the lifetime of the process, and I think there isn't much\n>> point doing the \"free() immediately before exiting\".\n>\n> Yeah, sorry, I meant to say that more explicitly, but clearly didn't. I\n> think it is fine as-is.\n\nYeah, sorry, I didn't mean \"You are wrong and here is why\".  I was\nmerely agreeing with you.\n\nThanks for running grep over the codebase to check possible\nremaining problems.\n"}]}