[PATCH 1/1] scan-tree: handle error in git_config_from_file()

[John Keeping]

On Tue, Mar 08, 2016 at 02:51:46PM +0100, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> Signed-off-by: Christian Hesse <mail at eworm.de>

Is this solving a particular problem or did you just notice that the
return value is ignored?

I don't think returning when this fails is correct because we've already
added the repository to the list by this point and a lot of the
remaining code in this function will do something sensible even if
git_config_from_file() fails.

In fact, git_config_from_file() sets the die_on_error flag for
do_config_from() so the only case that gives us an error here is if the
config file cannot be opened.  I don't think it's unreasonable to print
an error if that happens but bailing out of the function at this point
is wrong.

> ---
>  scan-tree.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/scan-tree.c b/scan-tree.c
> index 2e87999..bc17d14 100644
> --- a/scan-tree.c
> +++ b/scan-tree.c
> @@ -121,7 +121,11 @@ static void add_repo(const char *base, struct strbuf *path, repo_config_fn fn)
>  	config_fn = fn;
>  	if (ctx.cfg.enable_git_config) {
>  		strbuf_addstr(path, "config");
> -		git_config_from_file(gitconfig_config, path->buf, NULL);
> +		if (git_config_from_file(gitconfig_config, path->buf, NULL)) {
> +			fprintf(stderr, "Error reading config %s: %s (%d)\n",
> +				path->buf, strerror(errno), errno);
> +			return;
> +		}
>  		strbuf_setlen(path, pathlen);
>  	}
>  
> -- 
> 2.7.2