Skip to contentSkip to conversation

http-push: refactor request url creation

PATCH In review Tay Ray Chuan 3 participants 4 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 http-push: refactor request url creation

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.

---

* 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(-)
http-push.c +25 -37
index 59037df..816e658 100644
@@ -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;
Signature · 1 line
1.6.1.1.241.gc53a6.dirty
#1 lore raw

Hi,

Quoted 3 lines · Tay Ray Chuan

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 :-)

Quoted 2 lines

+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

#2 lore raw
Quoted 7 lines · Johannes Schindelin
Quoted 3 lines · Tay Ray Chuan

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?

#3 lore raw
Quoted 11 lines · Junio C Hamano
Quoted 7 lines · Johannes Schindelin
Quoted 3 lines · Tay Ray Chuan

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?

Signature · 2 lines
Cheers,
Ray Chuan