Skip to contentSkip to conversation

Fixes for dl/stash-show-untracked

PATCH In review Denton Liu 2 participants 4 messages May 12, 2021

lore
Share & tools
lore

Denton Liu

author
#0 lore raw

cover Fixes for dl/stash-show-untracked

These follow-up patches for the topic should be merged before the next release happens. In particular, the last patch fixes a potential segfault that occurs.

Denton Liu (2):
t3905: correct test title
stash show: fix segfault with --{include,only}-untracked

 builtin/stash.c                    |  8 ++++++--
 t/t3905-stash-include-untracked.sh | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 3 deletions(-)
Signature · 1 line
2.31.1.751.gd2f1c929bd
author
#2 lore raw

PATCH 2/2 stash show: fix segfault with --{include,only}-untracked

When git stash show --include-untracked or
git stash show --only-untracked is run on a stash that doesn't include an untracked entry, a segfault occurs. This happens because we do not check whether the untracked entry is actually present and just attempt to blindly dereference it.

Ensure that the untracked entry is present before actually attempting to dereference it.

Signed-off-by Denton Liu <[email protected]>
---
 builtin/stash.c                    |  8 ++++++--
 t/t3905-stash-include-untracked.sh | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
builtin/stash.c +6 -2
index 8922a1240c..82e4829d44 100644
@@ -900,10 +900,14 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
 		break;
 	case UNTRACKED_ONLY:
-		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
+		if (info.has_u)
+			diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
 		break;
 	case UNTRACKED_INCLUDE:
-		diff_include_untracked(&info, &rev.diffopt);
+		if (info.has_u)
+			diff_include_untracked(&info, &rev.diffopt);
+		else
+			diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
 		break;
 	}
 	log_tree_diff_flush(&rev);
t/t3905-stash-include-untracked.sh +15 -0
index 2e6796725b..1c9765928d 100755
@@ -405,4 +405,19 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '
 	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
 '
 
+test_expect_success 'stash show --{include,only}-untracked on stashes without untracked entries' '
+	git reset --hard &&
+	git clean -xf &&
+	>tracked &&
+	git add tracked &&
+	git stash &&
+
+	git stash show >expect &&
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	git stash show --only-untracked >actual &&
+	test_must_be_empty actual
+'
+
 test_done
Signature · 1 line
2.31.1.751.gd2f1c929bd
author
#1 lore raw

PATCH 1/2 t3905: correct test title

In reply to Denton Liu

We reference the non-existent option git stash show --show-untracked when we really meant --only-untracked. Correct the test title accordingly.

Signed-off-by Denton Liu <[email protected]>
---
 t/t3905-stash-include-untracked.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t3905-stash-include-untracked.sh +1 -1
index b470db7ef7..2e6796725b 100755
@@ -367,7 +367,7 @@ test_expect_success 'stash show --only-untracked only shows untracked files' '
 	test_cmp expect actual
 '
 
-test_expect_success 'stash show --no-include-untracked cancels --{include,show}-untracked' '
+test_expect_success 'stash show --no-include-untracked cancels --{include,only}-untracked' '
 	git reset --hard &&
 	git clean -xf &&
 	>untracked &&
Signature · 1 line
2.31.1.751.gd2f1c929bd
#3 lore raw

Re: [PATCH 2/2] stash show: fix segfault with --{include,only}-untracked

In reply to Denton Liu
Quoted 8 lines · Denton Liu

When git stash show --include-untracked or
git stash show --only-untracked is run on a stash that doesn't include an untracked entry, a segfault occurs. This happens because we do not check whether the untracked entry is actually present and just attempt to blindly dereference it.

Ensure that the untracked entry is present before actually attempting to dereference it.

Makes sense. Thanks.

Quoted 52 lines
Signed-off-by Denton Liu <[email protected]>
---
 builtin/stash.c                    |  8 ++++++--
 t/t3905-stash-include-untracked.sh | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
builtin/stash.c +6 -2
index 8922a1240c..82e4829d44 100644
@@ -900,10 +900,14 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
 		break;
 	case UNTRACKED_ONLY:
-		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
+		if (info.has_u)
+			diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
 		break;
 	case UNTRACKED_INCLUDE:
-		diff_include_untracked(&info, &rev.diffopt);
+		if (info.has_u)
+			diff_include_untracked(&info, &rev.diffopt);
+		else
+			diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
 		break;
 	}
 	log_tree_diff_flush(&rev);
t/t3905-stash-include-untracked.sh +15 -0
index 2e6796725b..1c9765928d 100755
@@ -405,4 +405,19 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '
 	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
 '
 
+test_expect_success 'stash show --{include,only}-untracked on stashes without untracked entries' '
+	git reset --hard &&
+	git clean -xf &&
+	>tracked &&
+	git add tracked &&
+	git stash &&
+
+	git stash show >expect &&
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	git stash show --only-untracked >actual &&
+	test_must_be_empty actual
+'
+
 test_done