Hi,
On Thu, 5 Jun 2008, David ʽBombeʼ Roden wrote:
> From 1f55b9176248673b78c77ff292fb5c6c988a7f8b Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?David=20=E2=80=98Bombe=E2=80=99=20Roden?= <bombe@xxxxxxxxxxxxxxxxx>
> Date: Thu, 5 Jun 2008 00:53:56 +0200
> Subject: [PATCH] handle http urls with query string ("?foo") correctly
Please follow the hints in Documentation/SubmittingPatches, or just
imitate others' mails with patches.
> I'm a developer for the Freenet Project[1] and tried to get git running
> with Freenet's http-gateway. For several reasons I have to use URLs like
> "http://host:8888/USK@<lots-of-stuff>/project.git/4?type=text/plain".
> This breaks the way git creates http URLs to retrieve. The following
> patch remedys this situation by inserting the git-generated part of the
> URL (like "/info/refs") before the query string.
I think that this paragraph should be rewritten into a less "I did" form,
and be the commit message.
> diff --git a/http-walker.c b/http-walker.c
> index 99f397e..711e55b 100644
> --- a/http-walker.c
> +++ b/http-walker.c
> @@ -100,7 +100,6 @@ static void start_object_request(struct walker *walker,
> char *hex = sha1_to_hex(obj_req->sha1);
> char prevfile[PATH_MAX];
> char *url;
> - char *posn;
> int prevlocal;
> unsigned char prev_buf[PREV_BUF_SIZE];
> ssize_t prev_read = 0;
> @@ -146,16 +145,11 @@ static void start_object_request(struct walker *walker,
>
> SHA1_Init(&obj_req->c);
>
> - url = xmalloc(strlen(obj_req->repo->base) + 51);
> + char *suffix = xmalloc(51);
I guess you are a Java programmer? In C, this would be "char
suffix[51];".
Also, we like to have C89 compatibility, especially when it is too simple:
declarations have to come _before_ statements.
> obj_req->url = xmalloc(strlen(obj_req->repo->base) + 51);
> - strcpy(url, obj_req->repo->base);
> - posn = url + strlen(obj_req->repo->base);
> - strcpy(posn, "/objects/");
> - posn += 9;
> - memcpy(posn, hex, 2);
> - posn += 2;
> - *(posn++) = '/';
> - strcpy(posn, hex + 2);
> + strcpy(suffix, "/objects/");
> + sprintf(suffix + 9, "%c%c/%s", hex[0], hex[1], hex + 2);
> + url = transform_url(obj_req->repo->base, suffix);
This part of Git's source code predates the strbuf work, and therefore it
is understandable that strbufs are not used there. However, I think that
your changes just cry for want of strbufs.
Also, the strcpy() could be merged into the sprintf().
> @@ -384,8 +378,9 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch
> if (walker->get_verbosely)
> fprintf(stderr, "Getting index for pack %s\n", hex);
>
> - url = xmalloc(strlen(repo->base) + 64);
> - sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
> + char *index_url = xmalloc(64);
> + sprintf(index_url, "/objects/pack/pack-%s.idx", hex);
> + url = transform_url(repo->base, index_url);
Again, declaration-after-statement, and again an opportunity for an
strbuf.
Maybe you want to rewrite your patch before I keep on repeating these two
comments? ;-)
Thanks,
Dscho
