All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Sergey Andreenko <andreenkosa@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: [PATCH] diff: don't attempt to strip prefix from absolute Windows paths
Date: Thu, 18 Oct 2018 20:38:25 +0200	[thread overview]
Message-ID: <ae6fc699-6e09-2979-40dc-9cc49f4f8365@kdbg.org> (raw)
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

  reply	other threads:[~2018-10-18 18:38 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-12 18:23 issue: strange `git diff --numstat` behavior Sergey Andreenko
2018-10-13  8:13 ` Junio C Hamano
2018-10-18 18:38   ` Johannes Sixt [this message]
2018-10-18 18:49     ` [PATCH] diff: don't attempt to strip prefix from absolute Windows paths Stefan Beller
2018-10-18 19:11       ` Johannes Sixt
2018-10-19 16:58         ` [PATCH v2] " Johannes Sixt
2018-10-19 17:04           ` Stefan Beller
2018-10-19  1:34     ` [PATCH] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae6fc699-6e09-2979-40dc-9cc49f4f8365@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=andreenkosa@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH] diff: don'\''t attempt to strip prefix from absolute Windows paths' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.