Re: [PATCH 6/9] http: update base URLs when we see redirects

On Sun, Sep 29, 2013 at 03:26:45PM -0400, Eric Sunshine wrote:

On Sat, Sep 28, 2013 at 4:34 AM, Jeff King [off-list ref] wrote:

quoted

diff --git a/http.c b/http.c
index 65a0048..8775b5c 100644
--- a/http.c
+++ b/http.c

@@ -921,11 +921,71 @@ static int http_request_reauth(const char *url,
+static int update_url_from_redirect(struct strbuf *base,
+                                   const char *asked,
+                                   const struct strbuf *got)
+{
+       const char *tail;
+       size_t tail_len;
+
+       if (!strcmp(asked, got->buf))
+               return 0;
+
+       if (strncmp(asked, base->buf, base->len))
+               die("BUG: update_url_from_redirect: %s is not a superset of %s",
+                   asked, base->buf);

Is there something non-obvious going on here? die(...,base->buf) takes
advantage of the terminating NUL promised by strbuf, but then
strncmp(...,base->buf,base->len) is used rather than the simpler
strcmp(...,base->buf).

Yes, we are not checking for equality, but rather making sure that
"asked" begins with "base->buf". It might be more clearly written as:

  if (prefixcmp(asked, base->buf))

I was trying to take advantage of the fact that we know base->len
already, but this it not a particularly performance-critical code path.
We can afford the extra strlen that prefixcmp will do.

-Peff