Am 13.01.2016 um 23:03 schrieb Junio C Hamano:
Stefan Beller <sbeller@xxxxxxxxxx> writes:+ while (url) { + if (starts_with_dot_dot_slash(url)) { + char *rfind; + url += 3; + + rfind = last_dir_separator(remoteurl); + if (rfind) + *rfind = '\0'; + else { + rfind = strrchr(remoteurl, ':'); + if (rfind) { + *rfind = '\0'; + colonsep = 1; + } else { + if (is_relative || !strcmp(".", remoteurl)) + die(_("cannot strip one component off url '%s'"), remoteurl); + else + remoteurl = xstrdup("."); + } + }It is somewhat hard to see how this avoids stripping one (or both) slashes just after "http:" in remoteurl="http://site/path/";, leaving just "http:/" (or "http:"). This codepath has overly deep nesting levels. Is this the simplest we can do?
The code as written is quite easy to follow when compared to the original shell code. I think that is a reasonable goal, and improvements can into separate patches.
The final else { if .. else } can be made into else if .. else to dedent the overlong die() by one level, but I am wondering if the deep nesting is just a symptom of logic being unnecessarily complex.+ } else if (starts_with_dot_slash(url)) { + url += 2; + } else + break; + }
For example, the section that begins here...
+ 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); + if (!up_path || !is_relative) + return out; + + strbuf_addf(&sb, "%s%s", up_path, out); + free(out); + return strbuf_detach(&sb, NULL);
... and ends here can easily be rewritten to become a single strbuf_addf() without the xstrdup()s and without the early exit (at the cost of some additional ?: conditionals in the arguments).
-- Hannes -- 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