[PATCH] git-config: print error message if the config file cannot be read

[PATCH] git-config: print error message if the config file cannot be read

[Gerrit Pape]

Instead of simply exiting with 255, print an error message including
the reason why the config file cannot be opened or read.

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(-)

diff --git a/builtin-config.c b/builtin-config.c
index 1bb0ebb..750a403 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -235,8 +235,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		argv++;
 	}
 
-	if (show_all)
-		return git_config(show_all_config);
+	if (show_all) {
+		if (git_config(show_all_config) == -1)
+			die("unable to read config file %s: %s",
+			    getenv(CONFIG_ENVIRONMENT), strerror(errno));
+		return 0;
+	}
 	switch (argc) {
 	case 2:
 		return get_value(argv[1], NULL);
-- 
1.5.3.4

Re: [PATCH] git-config: print error message if the config file cannot be read

[Johannes Schindelin]

Hi,

On Tue, 9 Oct 2007, Gerrit Pape wrote:

> +		if (git_config(show_all_config) == -1)

I'd rather check for < 0, just for future proofing.

Ciao,
Dscho

Re: [PATCH] git-config: print error message if the config file cannot be read

[Johannes Sixt]

Gerrit Pape schrieb:
> @@ -235,8 +235,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  		argv++;
>  	}
>  
> -	if (show_all)
> -		return git_config(show_all_config);
> +	if (show_all) {
> +		if (git_config(show_all_config) == -1)
> +			die("unable to read config file %s: %s",
> +			    getenv(CONFIG_ENVIRONMENT), strerror(errno));

I don't think that this works well: If there are no config files at all, 
then we don't want to see an error - just as if the config file were empty.

Also, I don't think that errno is reliable at this point.

You probably want to see an error message *only* if you have supplied a file 
name with --file.

-- Hannes

[PATCH amend] git-config: print error message if the config file cannot be read

[Gerrit Pape]

Instead of simply exiting with 255, print an error message including
the reason why a config file specified through --file cannot be opened
or read.

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

On Tue, Oct 09, 2007 at 02:16:41PM +0100, Johannes Schindelin wrote:
> On Tue, 9 Oct 2007, Gerrit Pape wrote:
> > +           if (git_config(show_all_config) == -1)
> I'd rather check for < 0, just for future proofing.

Okay.

On Tue, Oct 09, 2007 at 03:30:46PM +0200, Johannes Sixt wrote:
> Gerrit Pape schrieb:
> >+    if (show_all) {
> >+            if (git_config(show_all_config) == -1)
> >+                    die("unable to read config file %s: %s",
> >+                        getenv(CONFIG_ENVIRONMENT), strerror(errno));
> I don't think that this works well: If there are no config files at
> all, then we don't want to see an error - just as if the config file
> were empty.
>
> Also, I don't think that errno is reliable at this point.
>
> You probably want to see an error message *only* if you have supplied
> a file name with --file.

I changed the patch to die() only if --file was sepcified, and errno is
not 0.

Thanks, Gerrit.


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

diff --git a/builtin-config.c b/builtin-config.c
index 4444d52..e5bf791 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -172,8 +172,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			type = T_INT;
 		else if (!strcmp(argv[1], "--bool"))
 			type = T_BOOL;
-		else if (!strcmp(argv[1], "--list") || !strcmp(argv[1], "-l"))
-			return git_config(show_all_config);
+		else if (!strcmp(argv[1], "--list") || !strcmp(argv[1], "-l")) {
+			if (git_config(show_all_config) < 0 && file && errno)
+				die("unable to read config file %s: %s", file,
+				    strerror(errno));
+			return 0;
+		}
 		else if (!strcmp(argv[1], "--global")) {
 			char *home = getenv("HOME");
 			if (home) {
-- 
1.5.3.4

Re: [PATCH amend] git-config: print error message if the config file cannot be read

[Johannes Sixt]

Gerrit Pape schrieb:
>> You probably want to see an error message *only* if you have supplied
>> a file name with --file.
> 
> I changed the patch to die() only if --file was sepcified, and errno is
> not 0.

Thanks. I can live with that. ;)

-- Hannes