Re: [PATCH] ls-files: fix "--format" output of relative paths
- From
ZheNing Hu <adlternative@gmail.com>
- Date
- Mar 10, 2023, 13:40 UTC
- Message-ID
- <CAOLTT8QKCoCdi8hBmrOYNchFEFKZ_MgPiOZY0N0VkE9RK07CbQ@mail.gmail.com>
- In-Reply-To
- <pull.1465.git.git.1678453473484.gitgitgadget@gmail.com>
Adam Johnson via GitGitGadget <gitgitgadget@gmail.com> 于2023年3月10日周五 21:04写道:
Show 27 quoted lines
> > 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!
Show 28 quoted 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/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.
Show 32 quoted lines
> 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