[PATCH v2] http-push: refactor request url creation

[PATCH v2] http-push: refactor request url creation

[Tay Ray Chuan]

Currently, functions that deal with objects on the remote repository have to allocate and
do strcpys to generate the URL.

This patch saves them this trouble, by providing two functions, "append_remote_object_url"
and "get_remote_object_url".

Both generate a URL, with either the object's 2-digit hex directory (eg. /objects/a1/), or
the complete object location (eg. /objects/a1/b2).

However, they differ in that "append_remote_object_url" appends this URL to a strbuf, while
"get_remote_object_url" wraps around the former and returns the URL directly in char*.
Users usually would use "get_remote_object_url", but may find "append_remote_object_url"
useful if they require further string operations on the URL.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Acked-by: Junio C Hamano <gitster@pobox.com>
---

* renamed only_two_digit_postfix in original patch to only_two_digit_prefix
* rebased and generated on master (5dc1308)
* updated with Junio's changes (if (...) and fix memory leak)
* updated with Junio's double interface
* added back use of temporary string "url" in "start_fetch_loose"
* rebased and generated on master (a34a9db)

  http-push.c |   62 +++++++++++++++++++++++-----------------------------------
  1 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/http-push.c b/http-push.c
index 59037df..816e658 100644
--- a/http-push.c
+++ b/http-push.c
@@ -209,6 +209,20 @@ static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum d
  	return dav_headers;
  }

+static void append_remote_object_url(struct strbuf *buf, const char *url, const char *hex, int only_two_digit_prefix)
+{
+	strbuf_addf(buf, "%sobjects/%.*s/", url, 2, hex);
+	if (!only_two_digit_prefix)
+		strbuf_addf(buf, "%s", hex+2);
+}
+
+static char *get_remote_object_url(const char *url, const char *hex, int only_two_digit_prefix)
+{
+	struct strbuf buf = STRBUF_INIT;
+	append_remote_object_url(&buf, url, hex, only_two_digit_prefix);
+	return strbuf_detach(&buf, NULL);
+}
+
  static void finish_request(struct transfer_request *request);
  static void release_request(struct transfer_request *request);

@@ -255,7 +269,6 @@ static void start_fetch_loose(struct transfer_request *request)
  	char *filename;
  	char prevfile[PATH_MAX];
  	char *url;
-	char *posn;
  	int prevlocal;
  	unsigned char prev_buf[PREV_BUF_SIZE];
  	ssize_t prev_read = 0;
@@ -305,17 +318,8 @@ static void start_fetch_loose(struct transfer_request *request)

  	git_SHA1_Init(&request->c);

-	url = xmalloc(strlen(remote->url) + 50);
-	request->url = xmalloc(strlen(remote->url) + 50);
-	strcpy(url, remote->url);
-	posn = url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	*(posn++) = '/';
-	strcpy(posn, hex + 2);
-	strcpy(request->url, url);
+	url = get_remote_object_url(remote->url, hex, 0);
+	request->url = xstrdup(url);

  	/* If a previous temp file is present, process what was already
  	   fetched. */
@@ -388,16 +392,8 @@ static void start_mkcol(struct transfer_request *request)
  {
  	char *hex = sha1_to_hex(request->obj->sha1);
  	struct active_request_slot *slot;
-	char *posn;

-	request->url = xmalloc(strlen(remote->url) + 13);
-	strcpy(request->url, remote->url);
-	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	strcpy(posn, "/");
+	request->url = get_remote_object_url(remote->url, hex, 1);

  	slot = get_active_slot();
  	slot->callback_func = process_response;
@@ -512,7 +508,7 @@ static void start_put(struct transfer_request *request)
  {
  	char *hex = sha1_to_hex(request->obj->sha1);
  	struct active_request_slot *slot;
-	char *posn;
+	struct strbuf buf = STRBUF_INIT;
  	enum object_type type;
  	char hdr[50];
  	void *unpacked;
@@ -551,21 +547,13 @@ static void start_put(struct transfer_request *request)

  	request->buffer.buf.len = stream.total_out;

-	request->url = xmalloc(strlen(remote->url) +
-			       strlen(request->lock->token) + 51);
-	strcpy(request->url, remote->url);
-	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	*(posn++) = '/';
-	strcpy(posn, hex + 2);
-	request->dest = xmalloc(strlen(request->url) + 14);
-	sprintf(request->dest, "Destination: %s", request->url);
-	posn += 38;
-	*(posn++) = '_';
-	strcpy(posn, request->lock->token);
+	strbuf_addstr(&buf, "Destination: ");
+	append_remote_object_url(&buf, remote->url, hex, 0);
+	request->dest = strbuf_detach(&buf, NULL);
+
+	append_remote_object_url(&buf, remote->url, hex, 0);
+	strbuf_addstr(&buf, request->lock->token);
+	request->url = strbuf_detach(&buf, NULL);

  	slot = get_active_slot();
  	slot->callback_func = process_response;
-- 
1.6.1.1.241.gc53a6.dirty

Re: [PATCH v2] http-push: refactor request url creation

[Johannes Schindelin]

Hi,

On Thu, 29 Jan 2009, Tay Ray Chuan wrote:

> Currently, functions that deal with objects on the remote repository have to
> allocate and
> do strcpys to generate the URL.

That is a funny way to wrap the commit message :-)

> +static void append_remote_object_url(struct strbuf *buf, const char *url,
> const char *hex, int only_two_digit_prefix)

Here, you still have a corrupt patch.  Which might be helped by wrapping 
the overly long line in the first place?

Ciao,
Dscho

Re: [PATCH v2] http-push: refactor request url creation

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Thu, 29 Jan 2009, Tay Ray Chuan wrote:
>
>> Currently, functions that deal with objects on the remote repository have to
>> allocate and
>> do strcpys to generate the URL.
>
> That is a funny way to wrap the commit message :-)

That's format=flowed in action, isn't it?

Re: [PATCH v2] http-push: refactor request url creation

[Tay Ray Chuan]

On Fri, Jan 30, 2009 at 1:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> On Thu, 29 Jan 2009, Tay Ray Chuan wrote:
>>
>>> Currently, functions that deal with objects on the remote repository have to
>>> allocate and
>>> do strcpys to generate the URL.
>>
>> That is a funny way to wrap the commit message :-)
>
> That's format=flowed in action, isn't it?

Ok, I've turned that off.

Eh, no I didn't wrap anything, my message looks fine here:

http://permalink.gmane.org/gmane.comp.version-control.git/107691

...turning off format=flowed would fix that?

-- 
Cheers,
Ray Chuan