Re: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> +static char *relative_url(const char *remote_url,
> +				const char *url,
> +				const char *up_path)
> +{
> +	int is_relative = 0;
> +	int colonsep = 0;
> +	char *out;
> +	char *remoteurl = xstrdup(remote_url);
> +	struct strbuf sb = STRBUF_INIT;
> +	size_t len = strlen(remoteurl);
> +
> +	if (is_dir_sep(remoteurl[len]))
> +		remoteurl[len] = '\0';
> +
> +	if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
> +		is_relative = 0;
> +	else {
> +		is_relative = 1;
> +		/*
> +		 * Prepend a './' to ensure all relative
> +		 * remoteurls start with './' or '../'
> +		 */
> +		if (!starts_with_dot_slash(remoteurl) &&
> +		    !starts_with_dot_dot_slash(remoteurl)) {
> +			strbuf_reset(&sb);
> +			strbuf_addf(&sb, "./%s", remoteurl);
> +			free(remoteurl);
> +			remoteurl = strbuf_detach(&sb, NULL);
> +		}
> +	}
> +	/*
> +	 * When the url starts with '../', remove that and the
> +	 * last directory in remoteurl.
> +	 */
> +	while (url) {
> +		if (starts_with_dot_dot_slash(url)) {
> +			url += 3;
> +			colonsep |= chop_last_dir(&remoteurl, is_relative);
> +		} else if (starts_with_dot_slash(url))
> +			url += 2;
> +		else
> +			break;
> +	}
> +	strbuf_reset(&sb);
> +	strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
> +
> +	if (starts_with_dot_slash(sb.buf))
> +		out = xstrdup(sb.buf + 2);
> +	else
> +		out = xstrdup(sb.buf);
> +	strbuf_reset(&sb);
> +
> +	free(remoteurl);

This is a rather strange place to put this free(), as you are done
with it a bit earlier, but it's OK.  I briefly wondered if the code
becomes easier to follow with fewer free(remoteurl) if this local
variable is made into a strbuf, but I didn't seriously think it
through.

Otherwise looking good.

Thanks.