Re: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 28, 2016, 22:03 UTC
- Message-ID
- <xmqqzivpl5uo.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1453948237-9860-2-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 53 quoted lines
> +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.