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@xxxxxx>
> ---
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". But I think the
spirit of --informative-errors is to let that information leak, and this
is a good change.
> -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.
One of:
errno = EACCESS;
return -1;
or
return -EACCESS;
would be more idiomatic, I think.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html