Re: [PATCH 2/3] fixup for patch 2: actually check the return value

[Tanay Abhra <tanayabh@gmail.com>]

I think it would be unnecessary for the current iteration.
Currently git_configset_add_file has only two possible return values
-1 or 0. I could add specialized error values for ENOENT or ENOTDIR
or EACCES, but the logs show that we silently ignore the first two.
I can add an access warn for the third. What do you think?

Thanks,
Tanay.

On 7/16/2014 9:39 PM, Matthieu Moy wrote:
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> I won't fight for this, but I think it makes sense.
> 
>  t/t1308-config-set.sh |  4 ++--
>  test-config.c         | 10 ++++++----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
> index ea031bf..f0307b7 100755
> --- a/t/t1308-config-set.sh
> +++ b/t/t1308-config-set.sh
> @@ -168,7 +168,7 @@ test_expect_success 'find value_list for a key from a configset' '
>  '
>  
>  test_expect_success 'proper error on non-existant files' '
> -	echo "Error reading configuration file non-existant-file." >expect &&
> +	echo "Error (-1) reading configuration file non-existant-file." >expect &&
>  	test_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&
>  	test_cmp expect actual
>  '
> @@ -176,7 +176,7 @@ test_expect_success 'proper error on non-existant files' '
>  test_expect_success 'proper error on non-accessible files' '
>  	chmod -r .git/config &&
>  	test_when_finished "chmod +r .git/config" &&
> -	echo "Error reading configuration file .git/config." >expect &&
> +	echo "Error (-1) reading configuration file .git/config." >expect &&
>  	test_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&
>  	test_cmp expect actual
>  '
> diff --git a/test-config.c b/test-config.c
> index cad35f4..9dd1b22 100644
> --- a/test-config.c
> +++ b/test-config.c
> @@ -86,8 +86,9 @@ int main(int argc, char **argv)
>  		}
>  	} else if (!strcmp(argv[1], "configset_get_value")) {
>  		for (i = 3; i < argc; i++) {
> -			if (git_configset_add_file(&cs, argv[i])) {
> -				fprintf(stderr, "Error reading configuration file %s.\n", argv[i]);
> +			int err;
> +			if ((err = git_configset_add_file(&cs, argv[i]))) {
> +				fprintf(stderr, "Error (%d) reading configuration file %s.\n", err, argv[i]);
>  				goto exit2;
>  			}
>  		}
> @@ -103,8 +104,9 @@ int main(int argc, char **argv)
>  		}
>  	} else if (!strcmp(argv[1], "configset_get_value_multi")) {
>  		for (i = 3; i < argc; i++) {
> -			if (git_configset_add_file(&cs, argv[i])) {
> -				fprintf(stderr, "Error reading configuration file %s.\n", argv[i]);
> +			int err;
> +			if ((err = git_configset_add_file(&cs, argv[i]))) {
> +				fprintf(stderr, "Error (%d) reading configuration file %s.\n", err, argv[i]);
>  				goto exit2;
>  			}
>  		}
>