| Message ID | pull.1465.git.git.1678453473484.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cfb62dd006ae82dd1e06fb177095c8885b40b1c3 |
| Headers | show |
| Series | ls-files: fix "--format" output of relative paths | expand |
Adam Johnson via GitGitGadget <gitgitgadget@gmail.com> 于2023年3月10日周五 21:04写道： > > From: Adam Johnson <me@adamj.eu> > > 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 <me@adamj.eu> > --- > 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. > Good catch! > 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(-) > > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index 4cf8a236483..02b9bbe7eb4 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -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); > } > Here I misused the outer strbuf. > static const char *get_tag(const struct cache_entry *ce, const char *tag) > diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh > index efb7450bf1e..ef6fb53f7f1 100755 > --- a/t/t3013-ls-files-format.sh > +++ b/t/t3013-ls-files-format.sh > @@ -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 > -- > gitgitgadget I'm glad you caught that mistake. Thanks. -- ZheNing Hu
"Adam Johnson via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Adam Johnson <me@adamj.eu> > > 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.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 4cf8a236483..02b9bbe7eb4 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -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) diff --git a/t/t3013-ls-files-format.sh b/t/t3013-ls-files-format.sh index efb7450bf1e..ef6fb53f7f1 100755 --- a/t/t3013-ls-files-format.sh +++ b/t/t3013-ls-files-format.sh @@ -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 &&