{"thread":{"id":"59374","subject":"[PATCH] ls-files: fix \"--format\" output of relative paths","startedAt":"2023-03-10T13:04:39Z","lastAt":"2023-03-10T17:40:18Z","messageCount":3,"participants":["Adam Johnson via GitGitGadget","ZheNing Hu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"473354","messageId":"pull.1465.git.git.1678453473484.gitgitgadget@gmail.com","threadId":"59374","inReplyTo":null,"subject":"[PATCH] ls-files: fix \"--format\" output of relative paths","fromName":"Adam Johnson via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-10T13:04:33Z","receivedAt":"2023-03-10T13:04:39Z","isPatch":true,"sender":{"key":"me@adamj.eu","avatar":"https://avatars.githubusercontent.com/u/857609?v=4"},"body":"From: Adam Johnson \n\nFix a bug introduced with the \"--format\" option in\nce74de931d, where relative paths were computed\nusing the output buffer, which could lead to\nrandom data in the output.\n\nSigned-off-by: Adam Johnson \n---\n ls-files: fix \"--format\" output of relative paths\n \n ce74de931d introduced with the \"--format\" option to ls-files. This\n commit had a bug: using --format='%(path)' with the \"top\" pathspec from\n within a subdirectory would lead to random memory being added to the\n output. For example, within the Documentation/ directory in Git’s own\n repository:\n \n $ git ls-files --format='%(path)' ':/' | head -n 2\n ../.cirrus.yml�뻤��\n ../.clang-format�뻤��\n \n \n This is due to reuse of a string buffer for calculating the relative\n path. The attached patch fixes this by using a fresh buffer, the same\n pattern used in other places where relative paths are computed.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1465%2Fadamchainz%2Ffix-ls-files-format-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1465/adamchainz/fix-ls-files-format-v1\nPull-Request: https://github.com/git/git/pull/1465\n\n builtin/ls-files.c | 5 ++++-\n t/t3013-ls-files-format.sh | 16 ++++++++++++++++\n 2 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 4cf8a236483..02b9bbe7eb4 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -89,12 +89,15 @@ static void write_name(const char *name)\n \n static void write_name_to_buf(struct strbuf *sb, const char *name)\n {\n-\tconst char *rel = relative_path(name, prefix_len ? prefix : NULL, sb);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *rel = relative_path(name, prefix_len ? prefix : NULL, &buf);\n \n \tif (line_terminator)\n \t\tquote_c_style(rel, sb, NULL, 0);\n \telse\n \t\tstrbuf_addstr(sb, rel);\n+\n+\tstrbuf_release(&buf);\n }\n \n static const char *get_tag(const struct cache_entry *ce, const char *tag)\ndiff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh\nindex efb7450bf1e..ef6fb53f7f1 100755\n--- a/t/t3013-ls-files-format.sh\n+++ b/t/t3013-ls-files-format.sh\n@@ -54,6 +54,22 @@ test_expect_success 'git ls-files --format path v.s. -s' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git ls-files --format with relative path' '\n+\tcat >expect <<-\\EOF &&\n+\t../o1.txt\n+\t../o2.txt\n+\t../o3.txt\n+\t../o4.txt\n+\t../o5.txt\n+\t../o6.txt\n+\tEOF\n+\tmkdir sub &&\n+\tcd sub &&\n+\tgit ls-files --format=\"%(path)\" \":/\" >../actual &&\n+\tcd .. &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git ls-files --format with -m' '\n \techo change >o1.txt &&\n \tcat >expect <<-\\EOF &&\n\nbase-commit: 768bb238c4843bf52847773a621de4dffa6b9ab5\n-- \ngitgitgadget\n"},{"id":"473356","messageId":"CAOLTT8QKCoCdi8hBmrOYNchFEFKZ_MgPiOZY0N0VkE9RK07CbQ@mail.gmail.com","threadId":"59374","inReplyTo":"pull.1465.git.git.1678453473484.gitgitgadget@gmail.com","subject":"Re: [PATCH] ls-files: fix \"--format\" output of relative paths","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2023-03-10T13:40:04Z","receivedAt":"2023-03-10T13:40:32Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Adam Johnson via GitGitGadget 于2023年3月10日周五 21:04写道：\n>\n> From: Adam Johnson \n>\n> Fix a bug introduced with the \"--format\" option in\n> ce74de931d, where relative paths were computed\n> using the output buffer, which could lead to\n> random data in the output.\n>\n> Signed-off-by: Adam Johnson \n> ---\n> ls-files: fix \"--format\" output of relative paths\n>\n> ce74de931d introduced with the \"--format\" option to ls-files. This\n> commit had a bug: using --format='%(path)' with the \"top\" pathspec from\n> within a subdirectory would lead to random memory being added to the\n> output. For example, within the Documentation/ directory in Git’s own\n> repository:\n>\n> $ git ls-files --format='%(path)' ':/' | head -n 2\n> ../.cirrus.yml�뻤��\n> ../.clang-format�뻤��\n>\n>\n> This is due to reuse of a string buffer for calculating the relative\n> path. The attached patch fixes this by using a fresh buffer, the same\n> pattern used in other places where relative paths are computed.\n>\n\nGood catch!\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1465%2Fadamchainz%2Ffix-ls-files-format-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1465/adamchainz/fix-ls-files-format-v1\n> Pull-Request: https://github.com/git/git/pull/1465\n>\n> builtin/ls-files.c | 5 ++++-\n> t/t3013-ls-files-format.sh | 16 ++++++++++++++++\n> 2 files changed, 20 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n> index 4cf8a236483..02b9bbe7eb4 100644\n> --- a/builtin/ls-files.c\n> +++ b/builtin/ls-files.c\n> @@ -89,12 +89,15 @@ static void write_name(const char *name)\n>\n> static void write_name_to_buf(struct strbuf *sb, const char *name)\n> {\n> - const char *rel = relative_path(name, prefix_len ? prefix : NULL, sb);\n> + struct strbuf buf = STRBUF_INIT;\n> + const char *rel = relative_path(name, prefix_len ? prefix : NULL, &buf);\n>\n> if (line_terminator)\n> quote_c_style(rel, sb, NULL, 0);\n> else\n> strbuf_addstr(sb, rel);\n> +\n> + strbuf_release(&buf);\n> }\n>\n\nHere I misused the outer strbuf.\n\n> static const char *get_tag(const struct cache_entry *ce, const char *tag)\n> diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh\n> index efb7450bf1e..ef6fb53f7f1 100755\n> --- a/t/t3013-ls-files-format.sh\n> +++ b/t/t3013-ls-files-format.sh\n> @@ -54,6 +54,22 @@ test_expect_success 'git ls-files --format path v.s. -s' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'git ls-files --format with relative path' '\n> + cat >expect <<-\\EOF &&\n> + ../o1.txt\n> + ../o2.txt\n> + ../o3.txt\n> + ../o4.txt\n> + ../o5.txt\n> + ../o6.txt\n> + EOF\n> + mkdir sub &&\n> + cd sub &&\n> + git ls-files --format=\"%(path)\" \":/\" >../actual &&\n> + cd .. &&\n> + test_cmp expect actual\n> +'\n> +\n> test_expect_success 'git ls-files --format with -m' '\n> echo change >o1.txt &&\n> cat >expect <<-\\EOF &&\n>\n> base-commit: 768bb238c4843bf52847773a621de4dffa6b9ab5\n> --\n> gitgitgadget\n\n\nI'm glad you caught that mistake.\n\nThanks.\n--\nZheNing Hu\n"},{"id":"473370","messageId":"xmqq8rg47bgr.fsf@gitster.g","threadId":"59374","inReplyTo":"pull.1465.git.git.1678453473484.gitgitgadget@gmail.com","subject":"Re: [PATCH] ls-files: fix \"--format\" output of relative paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-10T17:39:00Z","receivedAt":"2023-03-10T17:40:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Adam Johnson via GitGitGadget\" writes:\n\n> From: Adam Johnson \n>\n> Fix a bug introduced with the \"--format\" option in\n> ce74de931d, where relative paths were computed\n> using the output buffer, which could lead to\n> random data in the output.\n\nGood find. Both the explanation and the patch text are crystal\nclear. Nicely done.\n\nI'll replace \"ce74de9313\" above with \"ce74de93 (ls-files: introduce\n\"--format\" option, 2022-07-23)\" while queuing, though.\n\n"}]}