From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Tue, 8 Mar 2016 14:11:35 +0000 Subject: [PATCH 1/1] scan-tree: handle error in git_config_from_file() In-Reply-To: <1457445106-17365-1-git-send-email-list@eworm.de> References: <1457445106-17365-1-git-send-email-list@eworm.de> Message-ID: <20160308141135.GC17523@serenity.lan> On Tue, Mar 08, 2016 at 02:51:46PM +0100, Christian Hesse wrote: > From: Christian Hesse > > Signed-off-by: Christian Hesse 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