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

John Keeping john at keeping.me.uk
Tue Mar 8 15:36:14 CET 2016


On Tue, Mar 08, 2016 at 03:26:23PM +0100, Christian Hesse wrote:
> John Keeping <john at keeping.me.uk> on Tue, 2016/03/08 14:11:
> > 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.
> 
> Ok, probably you are right...
> 
> Actually I do have a particular problem, but it is not solved by
> this patch. :-p Just stumbled and thought it is a good idea.
> 
> I have a repository that has a config with bad permissions, so http server's
> user can not read it. cgit does not print http headers and http server bails
> out with error 500. What path does it take?

Hmm... bad permissions should result in fopen(2) failing, which would
take the path altered in your patch.

Can you run cgit as the http server's user from a terminal like this:

	CGIT_CONFIG=/path/to/cgitrc QUERY_STRING=url=/ cgit

?  It might produce an error message that your http server isn't
logging.


More information about the CGit mailing list