threads / patch / 10210

patchgit-config: handle --file option with relative pathname properly

Subject: [PATCH] git-config: handle --file option with relative pathname properly

## tl;dr

5 messages between Oct 9, 2007 and Oct 12, 2007. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Gerrit Pape· Oct 9, 2007, 12:49 UTC · lore

When calling git-config not from the top level directory of a repository, it changes directory before trying to open the config file specified through the --file option, which then fails if the config file was specified by a relative pathname. This patch adjusts the pathname to the config file if applicable.

The problem was noticed by Joey Hess, reported through
 http://bugs.debian.org/445208

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 builtin-config.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

Show changes to builtin-config.c +6 −2

diff --git a/builtin-config.c b/builtin-config.c
index 0a605e0..c2708ba 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -165,7 +165,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = 0;
 	char* value;
-	setup_git_directory_gently(&nongit);
+	const char *name = setup_git_directory_gently(&nongit);
 
 	while (1 < argc) {
 		if (!strcmp(argv[1], "--int"))
@@ -189,7 +189,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		else if (!strcmp(argv[1], "--file") || !strcmp(argv[1], "-f")) {
 			if (argc < 3)
 				usage(git_config_set_usage);
-			setenv(CONFIG_ENVIRONMENT, argv[2], 1);
+			if (argv[2][0] == '/')
+				name = argv[2];
+			else
+				name = name ? prefix_filename(name, strlen(name), argv[2]) : argv[2];
+			setenv(CONFIG_ENVIRONMENT, name, 1);
 			argc--;
 			argv++;
 		}

-- 
1.5.3.4

Johannes Schindelin· Oct 9, 2007, 13:14 UTC · re: Gerrit Pape · lore

Re: [PATCH] git-config: handle --file option with relative pathname properly

Hi,

On Tue, 9 Oct 2007, Gerrit Pape wrote:

Show 6 quoted lines

> @@ -189,7 +189,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  		else if (!strcmp(argv[1], "--file") || !strcmp(argv[1], "-f")) {
>  			if (argc < 3)
>  				usage(git_config_set_usage);
> -			setenv(CONFIG_ENVIRONMENT, argv[2], 1);
> +			if (argv[2][0] == '/')

Please use is_absolute_path() instead.

Ciao, Dscho

Johannes Sixt· Oct 9, 2007, 13:20 UTC · re: Gerrit Pape · lore

Re: [PATCH] git-config: handle --file option with relative pathname properly

Gerrit Pape schrieb:

Show 6 quoted lines

> @@ -189,7 +189,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  		else if (!strcmp(argv[1], "--file") || !strcmp(argv[1], "-f")) {
>  			if (argc < 3)
>  				usage(git_config_set_usage);
> -			setenv(CONFIG_ENVIRONMENT, argv[2], 1);
> +			if (argv[2][0] == '/')

Please use is_absolute_path() here.

> +				name = argv[2];
> +			else
> +				name = name ? prefix_filename(name, strlen(name), argv[2]) : argv[2];

Can't you avoid this ternary here? There's already an 'if' with the same 'else' branch.

> +			setenv(CONFIG_ENVIRONMENT, name, 1);
>  			argc--;
>  			argv++;
>  		}

-- Hannes

Gerrit Pape· Oct 12, 2007, 11:32 UTC · re: Johannes Sixt · lore

[PATCH amend] git-config: handle --file option with relative pathname properly

The problem was noticed by Joey Hess, reported through
 http://bugs.debian.org/445208

Signed-off-by: Gerrit Pape <pape@smarden.org>
---

On Tue, Oct 09, 2007 at 03:20:36PM +0200, Johannes Sixt wrote:

Show 5 quoted lines

> Gerrit Pape schrieb:
> >-                    setenv(CONFIG_ENVIRONMENT, argv[2], 1);
> >+                    if (argv[2][0] == '/')
>
> Please use is_absolute_path() here.

Okay.

Show 7 quoted lines

> >+                            name = argv[2];
> >+                    else
> >+                            name = name ? prefix_filename(name,
> >strlen(name), argv[2]) : argv[2];
>
> Can't you avoid this ternary here? There's already an 'if' with the same
> 'else' branch.

Sure, thanks, Gerrit.

 builtin-config.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

Show changes to builtin-config.c +7 −2

diff --git a/builtin-config.c b/builtin-config.c
index 0a605e0..4444d52 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -165,7 +165,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = 0;
 	char* value;
-	setup_git_directory_gently(&nongit);
+	const char *file = setup_git_directory_gently(&nongit);
 
 	while (1 < argc) {
 		if (!strcmp(argv[1], "--int"))
@@ -189,7 +189,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		else if (!strcmp(argv[1], "--file") || !strcmp(argv[1], "-f")) {
 			if (argc < 3)
 				usage(git_config_set_usage);
-			setenv(CONFIG_ENVIRONMENT, argv[2], 1);
+			if (!is_absolute_path(argv[2]) && file)
+				file = prefix_filename(file, strlen(file),
+						       argv[2]);
+			else
+				file = argv[2];
+			setenv(CONFIG_ENVIRONMENT, file, 1);
 			argc--;
 			argv++;
 		}

-- 
1.5.3.4

Johannes Schindelin· Oct 12, 2007, 12:08 UTC · re: Gerrit Pape · lore

Re: [PATCH amend] git-config: handle --file option with relative pathname properly

Hi,

On Fri, 12 Oct 2007, Gerrit Pape wrote:

> -	setup_git_directory_gently(&nongit);
> +	const char *file = setup_git_directory_gently(&nongit);

One last nit (because I did not see it earlier): please call the variable "prefix", not "file".

Other than that, I think your patch is obviously correct.

Thanks, Dscho