diff mbox series

var: avoid a segmentation fault when `HOME` is unset

Message ID pull.1580.git.1693808487058.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 256a94ef6c8c0c94f9629a1ffe893577ccef8efd
Headers show
Series var: avoid a segmentation fault when `HOME` is unset | expand

Commit Message

Johannes Schindelin Sept. 4, 2023, 6:21 a.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The code introduced in 576a37fccbf (var: add attributes files locations,
2023-06-27) paid careful attention to use `xstrdup()` for pointers known
never to be `NULL`, and `xstrdup_or_null()` otherwise.

One spot was missed, though: `git_attr_global_file()` can return `NULL`,
when the `HOME` variable is not set (and neither `XDG_CONFIG_HOME`), a
scenario not too uncommon in certain server scenarios.

Fix this, and add a test case to avoid future regressions.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    var: avoid a segmentation fault when HOME is unset

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1580%2Fdscho%2Favoid-segfault-in-git-var-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1580/dscho/avoid-segfault-in-git-var-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1580

 builtin/var.c      | 2 +-
 t/t0007-git-var.sh | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)


base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40

Comments

brian m. carlson Sept. 4, 2023, 8:26 p.m. UTC | #1 
On 2023-09-04 at 06:21:26, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The code introduced in 576a37fccbf (var: add attributes files locations,
> 2023-06-27) paid careful attention to use `xstrdup()` for pointers known
> never to be `NULL`, and `xstrdup_or_null()` otherwise.
> 
> One spot was missed, though: `git_attr_global_file()` can return `NULL`,
> when the `HOME` variable is not set (and neither `XDG_CONFIG_HOME`), a
> scenario not too uncommon in certain server scenarios.
> 
> Fix this, and add a test case to avoid future regressions.

Looks good to me.  Thanks for the patch, and my apologies for the
oversight.
Johannes Schindelin Sept. 5, 2023, 10:58 a.m. UTC | #2 
Hi brian,

On Mon, 4 Sep 2023, brian m. carlson wrote:

> On 2023-09-04 at 06:21:26, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The code introduced in 576a37fccbf (var: add attributes files locations,
> > 2023-06-27) paid careful attention to use `xstrdup()` for pointers known
> > never to be `NULL`, and `xstrdup_or_null()` otherwise.
> >
> > One spot was missed, though: `git_attr_global_file()` can return `NULL`,
> > when the `HOME` variable is not set (and neither `XDG_CONFIG_HOME`), a
> > scenario not too uncommon in certain server scenarios.
> >
> > Fix this, and add a test case to avoid future regressions.
>
> Looks good to me.

Thank you for the review.

Ciao,
Johannes
diff mbox series

Patch

diff --git a/builtin/var.c b/builtin/var.c
index 74161bdf1c6..8cf7dd9e2e5 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -66,7 +66,7 @@  static char *git_attr_val_system(int ident_flag UNUSED)
 
 static char *git_attr_val_global(int ident_flag UNUSED)
 {
-	char *file = xstrdup(git_attr_global_file());
+	char *file = xstrdup_or_null(git_attr_global_file());
 	if (file) {
 		normalize_path_copy(file, file);
 		return file;
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index 8cb597f99c4..ff4fd9348cc 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -268,4 +268,13 @@  test_expect_success 'listing and asking for variables are exclusive' '
 	test_must_fail git var -l GIT_COMMITTER_IDENT
 '
 
+test_expect_success '`git var -l` works even without HOME' '
+	(
+		XDG_CONFIG_HOME= &&
+		export XDG_CONFIG_HOME &&
+		unset HOME &&
+		git var -l
+	)
+'
+
 test_done