git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] diff: don't attempt to strip prefix from absolute Windows paths

From
Johannes Sixt <j6t@kdbg.org>
Date
Oct 18, 2018, 18:38 UTC
Message-ID
<ae6fc699-6e09-2979-40dc-9cc49f4f8365@kdbg.org>
In-Reply-To
<xmqqr2gu8dsx.fsf@gitster-ct.c.googlers.com>

git diff can be invoked with absolute paths. Typically, this triggers the --no-index case. Then the absolute paths remain in the file names that are printed in the output.

There is one peculiarity, though: When the command is invoked from a a sub-directory in a repository, then it is attempted to strip the sub-directory from the beginning of relative paths. Yet, to detect a relative path the code just checks for an initial forward slash. This mistakes a Windows style path like D:/base as a relative path and the output looks like this, for example:

  D:\test\one>git -P diff --numstat D:\base D:\diff
  1       1       {ase => iff}/1.txt
where the correct output should be
  D:\test\one>git -P diff --numstat D:\base D:\diff
  1       1       D:/{base => diff}/1.txt

If the sub-directory where 'git diff' is invoked is sufficiently deep that the prefix becomes longer than the path to be printed, then the subsequent code even accesses the paths out of bounds!

Use is_absolute_path() to detect Windows style absolute paths.

One might wonder whether the check for a directory separator that is visible in the patch context should be changed from == '/' to is_dir_sep() or not. It turns out not to be necessary. That code only ever investigates paths that have undergone pathspec normalization, after which there are only forward slashes even on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index f0c7557b40..d18eb198f2 100644
--- a/diff.c
+++ b/diff.c
@@ -4267,12 +4267,12 @@ static void diff_fill_oid_info(struct diff_filespec *one)
 static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
 {
 	/* Strip the prefix but do not molest /dev/null and absolute paths */
-	if (*namep && **namep != '/') {
+	if (*namep && !is_absolute_path(*namep)) {
 		*namep += prefix_length;
 		if (**namep == '/')
 			++*namep;
 	}
-	if (*otherp && **otherp != '/') {
+	if (*otherp && !is_absolute_path(*otherp)) {
 		*otherp += prefix_length;
 		if (**otherp == '/')
 			++*otherp;
-- 
2.19.1.406.g1aa3f475f3
Previous: Junio C HamanoNext: Stefan Beller
Message 3 of 8 in “issue: strange `git diff --numstat` behavior.”
  1. Sergey AndreenkoOct 12, 2018
  2. Junio C HamanoOct 13, 2018
  3. diff: don't attempt to strip prefix from absolute Windows pathsJohannes Sixt, Oct 18, 2018
  4. Stefan BellerOct 18, 2018
  5. Johannes SixtOct 18, 2018
  6. diff: don't attempt to strip prefix from absolute Windows pathsJohannes Sixt, Oct 19, 2018
  7. Stefan BellerOct 19, 2018
  8. Junio C HamanoOct 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.