From: Thomas Gummerer <t.gummerer@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Simon Ruderich <simon@ruderich.org>,
git@vger.kernel.org, Mike Hommey <mh@glandium.org>,
Thomas Gummerer <t.gummerer@gmail.com>
Subject: [PATCH v2] blame: fix segfault on untracked files
Date: Mon, 29 Aug 2016 20:50:19 +0100 [thread overview]
Message-ID: <20160829195019.30876-1-t.gummerer@gmail.com> (raw)
In-Reply-To: <xmqqeg57bfcu.fsf@gitster.mtv.corp.google.com>
Since 3b75ee9 ("blame: allow to blame paths freshly added to the index",
2016-07-16) git blame also looks at the index to determine if there is a
file that was freshly added to the index.
cache_name_pos returns -pos - 1 in case there is no match is found, or
if the name matches, but the entry has a stage other than 0. As git
blame should work for unmerged files, it uses strcmp to determine
whether the name of the returned position matches, in which case the
file exists, but is merely unmerged, or if the file actually doesn't
exist in the index.
If the repository is empty, or if the file would lexicographically be
sorted as the last file in the repository, -cache_name_pos - 1 is
outside of the length of the active_cache array, causing git blame to
segfault. Guard against that, and die() normally to restore the old
behaviour.
Reported-by: Simon Ruderich <simon@ruderich.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
> The point of this fix is not that we show the exact error message,
> but we fail in a controlled manner. I think
>
> test_expect_success 'blame untracked file in empty repo' '
> >untracked &&
> test_must_fail git blame untracked
> '
>
> is sufficient.
Yeah, I agree with that.
Thanks for the review!
builtin/blame.c | 3 ++-
t/t8002-blame.sh | 5 +++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 7ec7823..a5bbf91 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2244,7 +2244,8 @@ static void verify_working_tree_path(struct commit *work_tree, const char *path)
pos = cache_name_pos(path, strlen(path));
if (pos >= 0)
; /* path is in the index */
- else if (!strcmp(active_cache[-1 - pos]->name, path))
+ else if (-1 - pos < active_nr &&
+ !strcmp(active_cache[-1 - pos]->name, path))
; /* path is in the index, unmerged */
else
die("no such path '%s' in HEAD", path);
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index ff09ace..ab79de9 100755
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -6,6 +6,11 @@ test_description='git blame'
PROG='git blame -c'
. "$TEST_DIRECTORY"/annotate-tests.sh
+test_expect_success 'blame untracked file in empty repo' '
+ >untracked &&
+ test_must_fail git blame untracked
+'
+
PROG='git blame -c -e'
test_expect_success 'blame --show-email' '
check_count \
--
2.10.0.rc2.246.g4fd2c60
next prev parent reply other threads:[~2016-08-29 19:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-27 17:53 Crash when using git blame on untracked file Simon Ruderich
2016-08-27 20:01 ` Thomas Gummerer
2016-08-29 18:27 ` Junio C Hamano
2016-08-29 19:50 ` Thomas Gummerer [this message]
2016-08-29 21:17 ` [PATCH v2] blame: fix segfault on untracked files 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=20160829195019.30876-1-t.gummerer@gmail.com \
--to=t.gummerer@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mh@glandium.org \
--cc=simon@ruderich.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).