Re: [PATCH 2/2] daemon: report permission denied error to clients

> On Mon, Oct 17, 2011 at 12:11:16AM +0200, Clemens Buchacher wrote:
> 
> > If passed an inaccessible url, git daemon returns the
> > following error:
> > 
> >  $ git clone git://host/repo
> >  fatal: remote error: no such repository: /repo
> > 
> > In case of a permission denied error, return the following
> > instead:
> > 
> >  fatal: remote error: permission denied: /repo
> > 
> > Signed-off-by: Clemens Buchacher <drizzd@aon.at>
> > ---
> 
> I like the intent. This actually does leak a little more information
> than the existing --informative-errors, as before you couldn't tell the
> difference between "not found" and "not exported".

> > -static char *path_ok(char *directory)
> > +static int path_ok(char *directory, const char **return_path)
> >  {
> >  	static char rpath[PATH_MAX];
> >  	static char interp_path[PATH_MAX];
> > @@ -120,13 +120,13 @@ static char *path_ok(char *directory)
> >  
> >  	if (daemon_avoid_alias(dir)) {
> >  		logerror("'%s': aliased", dir);
> > -		return NULL;
> > +		return -1;
> >  	}
> >  
> >  	if (*dir == '~') {
> >  		if (!user_path) {
> >  			logerror("'%s': User-path not allowed", dir);
> > -			return NULL;
> > +			return EACCES;
> 
> The new calling conventions for this function seem a little weird.  I
> would expect either "return negative, and set errno" for usual library
> code, or possibly "return negative error value". But "return -1, or a
> positive error code" seems unusual to me.