Skip to content

Avoid segmentation fault in git range-diff when diff.noprefix=true

Johannes Schindelin via GitGitGadget git 2 versions In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest1 patch 1 replyIn review
  2. v11 patch 6 repliesSuperseded

Changes in v2 from the cover letter

* Use a command-line option instead of a command-line config setting.
* Instead of forcing a prefix, force no prefix (and adjust the strip level).
* Fix typo "all" -> "call" in the commit message (and adjust it to reflect the other changes since v1, too).

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/1range-diff: internally force `diff.noprefix=true`range-diff.c t/t3206-range-diff.sh+6 -11 reply

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/1] Avoid segmentation fault in git range-diff when diff.noprefix=true

This PR was inspired by this bug report:
https://public-inbox.org/git/[email protected]/T/#me576615d7a151cf2ed46186c482fbd88f9959914

Changes since v1:

* Use a command-line option instead of a command-line config setting.
* Instead of forcing a prefix, force no prefix (and adjust the strip level).
* Fix typo "all" -> "call" in the commit message (and adjust it to reflect the other changes since v1, too).

Johannes Schindelin (1):
range-diff: internally force diff.noprefix=true

 range-diff.c          | 3 ++-
 t/t3206-range-diff.sh | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

base-commit: bc12974a897308fd3254cf0cc90319078fe45eea
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-373%2Fdscho%2Frange-diff-requires-diff-prefix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-373/dscho/range-diff-requires-diff-prefix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/373

Range-diff vs v1:

1: 1f84f92846 ! 1: 9993a607be range-diff: internally force diff.noprefix=false
@@ -1,6 +1,6 @@
Author: Johannes Schindelin <[email protected]>

- range-diff: internally force diff.noprefix=false
+ range-diff: internally force diff.noprefix=true

When parsing the diffs, range-diff expects to see the prefixes a/ and b/ in the diff headers.
@@ -9,25 +9,33 @@
diff.noprefix=true. As range-diff is not prepared for that situation, this will cause a segmentation fault.

- Let's avoid that by forcing diff.noprefix=false just for that all to
- git log that generates the diffs that range-diff wants to parse.
+ Let's avoid that by passing the --no-prefix option to the git log
+ process that generates the diffs that range-diff wants to parse.
+ And of course expect the output to have no prefixes, then.

- Noticed-by: Michal Suchánek <[email protected]>
+ Reported-by: Michal Suchánek <[email protected]>

diff --git a/range-diff.c b/range-diff.c
--- a/range-diff.c
+++ b/range-diff.c
@@
- int offset, len;
- size_t size;

-- argv_array_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
-+ argv_array_pushl(&cp.args, "-c", "diff.noprefix=false",
-+ "log", "--no-color", "-p", "--no-merges",
+ argv_array_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
"--reverse", "--date-order", "--decorate=no",
++ "--no-prefix",
/*
* Choose indicators that are not used anywhere
+ * else in diffs, but still look reasonable
+@@
+ if (!util->diff_offset)
+ util->diff_offset = buf.len;
+ line[len - 1] = '\n';
+- len = parse_git_diff_header(&root, &linenr, 1, line,
++ len = parse_git_diff_header(&root, &linenr, 0, line,
+ len, size, &patch);
+ if (len < 0)
+ die(_("could not parse git header '%.*s'"), (int)len, line);

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh --- a/t/t3206-range-diff.sh

Signature · 1 line
gitgitgadget