ls-files: fix "--format" output of relative paths
PATCH 1/1 ls-files: fix "--format" output of relative paths
From: Adam Johnson <[email protected]>
Fix a bug introduced with the "--format" option in ce74de931d, where relative paths were computed using the output buffer, which could lead to random data in the output.
---
ls-files: fix "--format" output of relative paths
ce74de931d introduced with the "--format" option to ls-files. This commit had a bug: using --format='%(path)' with the "top" pathspec from within a subdirectory would lead to random memory being added to the output. For example, within the Documentation/ directory in Git’s own repository:
$ git ls-files --format='%(path)' ':/' | head -n 2 ../.cirrus.yml�뻤��
../.clang-format�뻤��
This is due to reuse of a string buffer for calculating the relative path. The attached patch fixes this by using a fresh buffer, the same pattern used in other places where relative paths are computed.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1465%2Fadamchainz%2Ffix-ls-files-format-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1465/adamchainz/fix-ls-files-format-v1
Pull-Request: https://github.com/git/git/pull/1465
builtin/ls-files.c | 5 ++++- t/t3013-ls-files-format.sh | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-)
builtin/ls-files.c +4 -1
@@ -89,12 +89,15 @@ static void write_name(const char *name)static void write_name_to_buf (struct strbuf *sb,const char *name) { -const char *rel =relative_path (name, prefix_len ? prefix :NULL , sb); +struct strbuf buf =STRBUF_INIT ; +const char *rel =relative_path (name, prefix_len ? prefix :NULL , &buf);if (line_terminator)quote_c_style (rel, sb,NULL ,0 );else strbuf_addstr (sb, rel); + +strbuf_release (&buf); }static const char *get_tag (const struct cache_entry *ce,const char *tag)
t/t3013-ls-files-format.sh +16 -0
@@ -54,6 +54,22 @@ test_expect_success 'git ls-files --format path v.s. -s' ' test_cmp expect actual ' +test_expect_success 'git ls-files --format with relative path' ' + cat >expect <<-\EOF && + ../o1.txt + ../o2.txt + ../o3.txt + ../o4.txt + ../o5.txt + ../o6.txt + EOF + mkdir sub && + cd sub && + git ls-files --format="%(path)" ":/" >../actual && + cd .. && + test_cmp expect actual +' + test_expect_success 'git ls-files --format with -m' ' echo change >o1.txt && cat >expect <<-\EOF &&
base-commit: 768bb238c4843bf52847773a621de4dffa6b9ab5
Signature · 1 line
gitgitgadget
Quoted 18 lines · Adam Johnson via GitGitGadget <[email protected]> 于2023年3月10日周五 21:04写道：
From: Adam Johnson <[email protected]>
Fix a bug introduced with the "--format" option in
ce74de931d, where relative paths were computed using the output buffer, which could lead to random data in the output.Signed-off-by Adam Johnson <[email protected]>---
ls-files: fix "--format" output of relative paths
ce74de931dintroduced with the "--format" option to ls-files. This commit had a bug: using --format='%(path)' with the "top" pathspec from within a subdirectory would lead to random memory being added to the output. For example, within theDocumentation/directory in Git’s own repository:
Quoted 9 lines
$ git ls-files --format='%(path)' ':/' | head -n 2 ../.cirrus.yml�뻤��
../.clang-format�뻤��This is due to reuse of a string buffer for calculating the relative path. The attached patch fixes this by using a fresh buffer, the same pattern used in other places where relative paths are computed.
Good catch!
Quoted 18 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1465%2Fadamchainz%2Ffix-ls-files-format-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1465/adamchainz/fix-ls-files-format-v1
Pull-Request: https://github.com/git/git/pull/1465builtin/ls-files.c | 5 ++++- t/t3013-ls-files-format.sh | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-)builtin/ls-files.c +1
-1@@ -89,12 +89,15 @@ static void write_name(const char *name)static void write_name_to_buf (struct strbuf *sb,const char *name) { -const char *rel =relative_path (name, prefix_len ? prefix :NULL , sb); +struct strbuf buf =STRBUF_INIT ;
builtin/ls-files.c static void write_name(const char *name) · line 102
Quoted 10 lines
builtin/ls-files.c +3
-0+const char *rel =relative_path (name, prefix_len ? prefix :NULL , &buf);if (line_terminator)quote_c_style (rel, sb,NULL ,0 );else strbuf_addstr (sb, rel); + +strbuf_release (&buf); }
Here I misused the outer strbuf.
Quoted 29 lines
builtin/ls-files.c +0
-0static const char *get_tag (const struct cache_entry *ce,const char *tag)t/t3013-ls-files-format.sh +16
-0@@ -54,6 +54,22 @@ test_expect_success 'git ls-files --format path v.s. -s' ' test_cmp expect actual ' +test_expect_success 'git ls-files --format with relative path' ' + cat >expect <<-\EOF && + ../o1.txt + ../o2.txt + ../o3.txt + ../o4.txt + ../o5.txt + ../o6.txt + EOF + mkdir sub && + cd sub && + git ls-files --format="%(path)" ":/" >../actual && + cd .. && + test_cmp expect actual +' + test_expect_success 'git ls-files --format with -m' ' echo change >o1.txt && cat >expect <<-\EOF &&
Quoted 3 lines
base-commit:
768bb238c4843bf52847773a621de4dffa6b9ab5--
gitgitgadget
I'm glad you caught that mistake.
Thanks.
--
ZheNing Hu
Quoted 6 lines · Adam Johnson via GitGitGadget
From: Adam Johnson <[email protected]>
Fix a bug introduced with the "--format" option in
ce74de931d, where relative paths were computed using the output buffer, which could lead to random data in the output.
Good find. Both the explanation and the patch text are crystal clear. Nicely done.
I'll replace "ce74de9313" above with "ce74de93 (ls-files: introduce "--format" option, 2022-07-23)" while queuing, though.