Re: [PATCH 5/6] http-push: check path length before using it

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 19, 2010, 18:33 UTC
Message-ID: <20101119183339.GB26187@burratino>
In-Reply-To: <1290170790-2200-6-git-send-email-rctay89@gmail.com>

Tay Ray Chuan wrote:

> We use path_len to skip the base url/path, but we do not know for sure
> if path does indeed contain the base url/path. Check if this is so.

[...]

Show 11 quoted lines

> --- a/http-push.c
> +++ b/http-push.c
> @@ -1116,8 +1116,14 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
>  				}
>  			}
>  			if (path) {
> -				path += repo->path_len;
> -				ls->dentry_name = xstrdup(path);
> +				if (strncmp(path,
> +					    (repo->path ? repo->path : repo->url),
> +					    repo->path_len) == 0) {

Nits: the usual idiom is

 if (!strncmp(path, ...

to make it is obvious from looking at the left side that this is a test for equality. The parentheses around the second parameter are not needed here...

Show 5 quoted lines

> +					path += repo->path_len;
> +					ls->dentry_name = xstrdup(path);
> +				} else
> +					error("Parsed path '%s' does not match url: '%s'\n",
> +					      path, (repo->path ? repo->path : repo->url));

... or here.  So perhaps:

			if (path) {
				const char *url = repo->path;
				if (!url)
					url = repo->url;
				if (strncmp(path, url, repo->path_len)) {
					error("Parsed path '%s' does not match url: '%s'\n",
						path, url);
				} else {
					path += ...

By the way, is the error behavior correct? This prints a message with "error: " and then continues anyway.