{"thread":{"id":"56456","subject":"[PATCH] hash-object: prefix_filename() returns allocated memory these days","startedAt":"2021-09-07T22:10:29Z","lastAt":"2021-09-08T09:42:32Z","messageCount":2,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"434962","messageId":"xmqqsfyg82b5.fsf@gitster.g","threadId":"56456","inReplyTo":null,"subject":"[PATCH] hash-object: prefix_filename() returns allocated memory these days","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-07T22:10:22Z","receivedAt":"2021-09-07T22:10:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Back when a1be47e4 (hash-object: fix buffer reuse with --path in a\nsubdirectory, 2017-03-20) was written, the prefix_filename() helper\nused a static piece of memory to the caller, making the caller\nresponsible for copying it, if it wants to keep it across another\ncall to the same function.  Two callers of the prefix_filename() in\nhash-object were made to xstrdup() the value obtained from it.\n\nBut in the same series, when e4da43b1 (prefix_filename: return newly\nallocated string, 2017-03-20) changed the rule to gave the caller\npossession of the memory, we forgot to revert one of the xstrdup()\nchanges, allowing the returned value to leak.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * \"git show -W e4da43b1 builtin/hash-object.c\" made me curious.\n\n builtin/hash-object.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git c/builtin/hash-object.c w/builtin/hash-object.c\nindex 640ef4ded5..084cba204b 100644\n--- c/builtin/hash-object.c\n+++ w/builtin/hash-object.c\n@@ -117,7 +117,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)\n \t\tprefix = setup_git_directory_gently(&nongit);\n \n \tif (vpath && prefix)\n-\t\tvpath = xstrdup(prefix_filename(prefix, vpath));\n+\t\tvpath = prefix_filename(prefix, vpath);\n \n \tgit_config(git_default_config, NULL);\n \n"},{"id":"435046","messageId":"YTiFhNryAxCobH+m@coredump.intra.peff.net","threadId":"56456","inReplyTo":"xmqqsfyg82b5.fsf@gitster.g","subject":"Re: [PATCH] hash-object: prefix_filename() returns allocated memory these days","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-08T09:42:28Z","receivedAt":"2021-09-08T09:42:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 07, 2021 at 03:10:22PM -0700, Junio C Hamano wrote:\n\n> Back when a1be47e4 (hash-object: fix buffer reuse with --path in a\n> subdirectory, 2017-03-20) was written, the prefix_filename() helper\n> used a static piece of memory to the caller, making the caller\n> responsible for copying it, if it wants to keep it across another\n> call to the same function.  Two callers of the prefix_filename() in\n> hash-object were made to xstrdup() the value obtained from it.\n> \n> But in the same series, when e4da43b1 (prefix_filename: return newly\n> allocated string, 2017-03-20) changed the rule to gave the caller\n> possession of the memory, we forgot to revert one of the xstrdup()\n> changes, allowing the returned value to leak.\n\nGood catch. The patch looks obviously correct to me.\n\n-Peff\n"}]}