[PATCH 1/4] Cleanup variables in http.[ch]

[PATCH 1/4] Cleanup variables in http.[ch]

[Mike Hommey]

Quite some variables defined as extern in http.h are only used in http.c,
and some others, only defined in http.c, were not static. Cleanup that.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 http.c |   28 ++++++++++++++--------------
 http.h |   18 ------------------
 2 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/http.c b/http.c
index e4aa9c1..146f626 100644
--- a/http.c
+++ b/http.c
@@ -4,31 +4,31 @@ int data_received;
 int active_requests = 0;
 
 #ifdef USE_CURL_MULTI
-int max_requests = -1;
-CURLM *curlm;
+static int max_requests = -1;
+static CURLM *curlm;
 #endif
 #ifndef NO_CURL_EASY_DUPHANDLE
-CURL *curl_default;
+static CURL *curl_default;
 #endif
 char curl_errorstr[CURL_ERROR_SIZE];
 
-int curl_ssl_verify = -1;
-char *ssl_cert = NULL;
+static int curl_ssl_verify = -1;
+static char *ssl_cert = NULL;
 #if LIBCURL_VERSION_NUM >= 0x070902
-char *ssl_key = NULL;
+static char *ssl_key = NULL;
 #endif
 #if LIBCURL_VERSION_NUM >= 0x070908
-char *ssl_capath = NULL;
+static char *ssl_capath = NULL;
 #endif
-char *ssl_cainfo = NULL;
-long curl_low_speed_limit = -1;
-long curl_low_speed_time = -1;
-int curl_ftp_no_epsv = 0;
-char *curl_http_proxy = NULL;
+static char *ssl_cainfo = NULL;
+static long curl_low_speed_limit = -1;
+static long curl_low_speed_time = -1;
+static int curl_ftp_no_epsv = 0;
+static char *curl_http_proxy = NULL;
 
-struct curl_slist *pragma_header;
+static struct curl_slist *pragma_header;
 
-struct active_request_slot *active_queue_head = NULL;
+static struct active_request_slot *active_queue_head = NULL;
 
 size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer)
diff --git a/http.h b/http.h
index 72abac2..fe1b0d1 100644
--- a/http.h
+++ b/http.h
@@ -80,24 +80,6 @@ extern void http_cleanup(void);
 extern int data_received;
 extern int active_requests;
 
-#ifndef NO_CURL_EASY_DUPHANDLE
-extern CURL *curl_default;
-#endif
 extern char curl_errorstr[CURL_ERROR_SIZE];
 
-extern int curl_ssl_verify;
-extern char *ssl_cert;
-#if LIBCURL_VERSION_NUM >= 0x070902
-extern char *ssl_key;
-#endif
-#if LIBCURL_VERSION_NUM >= 0x070908
-extern char *ssl_capath;
-#endif
-extern char *ssl_cainfo;
-extern long curl_low_speed_limit;
-extern long curl_low_speed_time;
-
-extern struct curl_slist *pragma_header;
-extern struct curl_slist *no_range_header;
-
 #endif /* HTTP_H */
-- 
1.5.3.7

[PATCH 2/4] Use strbuf in http code

[Mike Hommey]

Also, replace whitespaces with tabs in some places

Signed-off-by: Mike Hommey <mh@glandium.org>
---

 While testing this, I noticed 3 things:
 - CURL_MULTI makes the code very racy
 - a lot of the code doesn't do anything useful without CURL_MULTI
 - the code is redundant

 So, if I have some time soon, I'll try to refactor all that.

 http-push.c   |  186 ++++++++++++++++++--------------------------------------
 http-walker.c |   59 +++++++------------
 http.c        |   34 ++++------
 http.h        |   11 ++--
 transport.c   |   18 ++----
 5 files changed, 107 insertions(+), 201 deletions(-)

diff --git a/http-push.c b/http-push.c
index 78283b4..e01b3a9 100644
--- a/http-push.c
+++ b/http-push.c
@@ -495,10 +495,10 @@ static void start_put(struct transfer_request *request)
 	memset(&stream, 0, sizeof(stream));
 	deflateInit(&stream, zlib_compression_level);
 	size = deflateBound(&stream, len + hdrlen);
-	request->buffer.buffer = xmalloc(size);
+	strbuf_init(&request->buffer.buf, size);
 
 	/* Compress it */
-	stream.next_out = request->buffer.buffer;
+	stream.next_out = (unsigned char *)request->buffer.buf.buf;
 	stream.avail_out = size;
 
 	/* First header.. */
@@ -515,8 +515,7 @@ static void start_put(struct transfer_request *request)
 	deflateEnd(&stream);
 	free(unpacked);
 
-	request->buffer.size = stream.total_out;
-	request->buffer.posn = 0;
+	request->buffer.buf.len = stream.total_out;
 
 	request->url = xmalloc(strlen(remote->url) +
 			       strlen(request->lock->token) + 51);
@@ -538,7 +537,7 @@ static void start_put(struct transfer_request *request)
 	slot->callback_func = process_response;
 	slot->callback_data = request;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &request->buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1003,18 +1002,13 @@ static int fetch_indices(void)
 {
 	unsigned char sha1[20];
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
 
 	struct active_request_slot *slot;
 	struct slot_results results;
 
-	data = xcalloc(1, 4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (push_verbosely)
 		fprintf(stderr, "Getting pack list\n");
 
@@ -1030,7 +1024,7 @@ static int fetch_indices(void)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			free(buffer.buffer);
+			strbuf_release(&buffer);
 			free(url);
 			if (results.http_code == 404)
 				return 0;
@@ -1038,18 +1032,18 @@ static int fetch_indices(void)
 				return error("%s", curl_errorstr);
 		}
 	} else {
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		free(url);
 		return error("Unable to start request");
 	}
 	free(url);
 
-	data = buffer.buffer;
-	while (i < buffer.posn) {
+	data = buffer.buf;
+	while (i < buffer.len) {
 		switch (data[i]) {
 		case 'P':
 			i++;
-			if (i + 52 < buffer.posn &&
+			if (i + 52 < buffer.len &&
 			    !prefixcmp(data + i, " pack-") &&
 			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
@@ -1064,7 +1058,7 @@ static int fetch_indices(void)
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 	return 0;
 }
 
@@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
 
 int fetch_ref(char *ref, unsigned char *sha1)
 {
-        char *url;
-        char hex[42];
-        struct buffer buffer;
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
 	char *base = remote->url;
 	struct active_request_slot *slot;
 	struct slot_results results;
-        buffer.size = 41;
-        buffer.posn = 0;
-        buffer.buffer = hex;
-        hex[41] = '\0';
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -1142,9 +1131,9 @@ int fetch_ref(char *ref, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
-        hex[40] = '\0';
-        get_sha1_hex(hex, sha1);
-        return 0;
+	buffer.buf[40] = '\0';
+	get_sha1_hex(buffer.buf, sha1);
+	return 0;
 }
 
 static void one_remote_object(const char *hex)
@@ -1267,10 +1256,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer out_buffer;
-	struct buffer in_buffer;
-	char *out_data;
-	char *in_data;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
+	struct strbuf in_buffer = STRBUF_INIT;
 	char *url;
 	char *ep;
 	char timeout_header[25];
@@ -1312,16 +1299,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 		ep = strchr(ep + 1, '/');
 	}
 
-	out_buffer.size = strlen(LOCK_REQUEST) + strlen(git_default_email) - 2;
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1, LOCK_REQUEST, git_default_email);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email);
 
 	sprintf(timeout_header, "Timeout: Second-%ld", timeout);
 	dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1330,7 +1308,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1354,8 +1332,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
 			XML_SetCharacterDataHandler(parser, xml_cdata);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 			if (result != XML_STATUS_OK) {
 				fprintf(stderr, "XML error: %s\n",
@@ -1369,8 +1347,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	}
 
 	curl_slist_free_all(dav_headers);
-	free(out_data);
-	free(in_data);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 
 	if (lock->token == NULL || lock->timeout <= 0) {
 		if (lock->token != NULL)
@@ -1521,10 +1499,8 @@ static void remote_ls(const char *path, int flags,
 	char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer in_buffer;
-	struct buffer out_buffer;
-	char *in_data;
-	char *out_data;
+	struct strbuf in_buffer = STRBUF_INIT;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	XML_Parser parser = XML_ParserCreate(NULL);
 	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
@@ -1540,16 +1516,7 @@ static void remote_ls(const char *path, int flags,
 
 	sprintf(url, "%s%s", remote->url, path);
 
-	out_buffer.size = strlen(PROPFIND_ALL_REQUEST);
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1, PROPFIND_ALL_REQUEST);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, PROPFIND_ALL_REQUEST);
 
 	dav_headers = curl_slist_append(dav_headers, "Depth: 1");
 	dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1557,7 +1524,7 @@ static void remote_ls(const char *path, int flags,
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1578,8 +1545,8 @@ static void remote_ls(const char *path, int flags,
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
 			XML_SetCharacterDataHandler(parser, xml_cdata);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 
 			if (result != XML_STATUS_OK) {
@@ -1594,8 +1561,8 @@ static void remote_ls(const char *path, int flags,
 
 	free(ls.path);
 	free(url);
-	free(out_data);
-	free(in_buffer.buffer);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 	curl_slist_free_all(dav_headers);
 }
 
@@ -1616,29 +1583,15 @@ static int locking_available(void)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer in_buffer;
-	struct buffer out_buffer;
-	char *in_data;
-	char *out_data;
+	struct strbuf in_buffer = STRBUF_INIT;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	XML_Parser parser = XML_ParserCreate(NULL);
 	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	int lock_flags = 0;
 
-	out_buffer.size =
-		strlen(PROPFIND_SUPPORTEDLOCK_REQUEST) +
-		strlen(remote->url) - 2;
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1,
-		 PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
 
 	dav_headers = curl_slist_append(dav_headers, "Depth: 0");
 	dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1646,7 +1599,7 @@ static int locking_available(void)
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1666,8 +1619,8 @@ static int locking_available(void)
 			XML_SetUserData(parser, &ctx);
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 
 			if (result != XML_STATUS_OK) {
@@ -1681,8 +1634,8 @@ static int locking_available(void)
 		fprintf(stderr, "Unable to start PROPFIND request\n");
 	}
 
-	free(out_data);
-	free(in_buffer.buffer);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 	curl_slist_free_all(dav_headers);
 
 	return lock_flags;
@@ -1800,30 +1753,20 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	char *out_data;
 	char *if_header;
-	struct buffer out_buffer;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	struct curl_slist *dav_headers = NULL;
-	int i;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
 	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 
-	out_buffer.size = 41;
-	out_data = xmalloc(out_buffer.size + 1);
-	i = snprintf(out_data, out_buffer.size + 1, "%s\n", sha1_to_hex(sha1));
-	if (i != out_buffer.size) {
-		fprintf(stderr, "Unable to initialize PUT request body\n");
-		return 0;
-	}
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
+	strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
 
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1834,7 +1777,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
-		free(out_data);
+		strbuf_release(&out_buffer.buf);
 		free(if_header);
 		if (results.curl_result != CURLE_OK) {
 			fprintf(stderr,
@@ -1844,7 +1787,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 			return 0;
 		}
 	} else {
-		free(out_data);
+		strbuf_release(&out_buffer.buf);
 		free(if_header);
 		fprintf(stderr, "Unable to start PUT request\n");
 		return 0;
@@ -2001,7 +1944,7 @@ static void mark_edges_uninteresting(struct commit_list *list)
 
 static void add_remote_info_ref(struct remote_ls_ctx *ls)
 {
-	struct buffer *buf = (struct buffer *)ls->userData;
+	struct strbuf *buf = (struct strbuf *)ls->userData;
 	unsigned char remote_sha1[20];
 	struct object *o;
 	int len;
@@ -2046,17 +1989,14 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
 
 static void update_remote_info_refs(struct remote_lock *lock)
 {
-	struct buffer buffer;
+	struct buffer buffer = { 0, STRBUF_INIT };
 	struct active_request_slot *slot;
 	struct slot_results results;
 	char *if_header;
 	struct curl_slist *dav_headers = NULL;
 
-	buffer.buffer = xcalloc(1, 4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
 	remote_ls("refs/", (PROCESS_FILES | RECURSIVE),
-		  add_remote_info_ref, &buffer);
+		  add_remote_info_ref, &buffer.buf);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
 		sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
@@ -2065,7 +2005,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		slot = get_active_slot();
 		slot->results = &results;
 		curl_easy_setopt(slot->curl, CURLOPT_INFILE, &buffer);
-		curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.posn);
+		curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.buf.len);
 		curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 		curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 		curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -2074,8 +2014,6 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
 		curl_easy_setopt(slot->curl, CURLOPT_URL, lock->url);
 
-		buffer.posn = 0;
-
 		if (start_active_slot(slot)) {
 			run_active_slot(slot);
 			if (results.curl_result != CURLE_OK) {
@@ -2086,7 +2024,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		}
 		free(if_header);
 	}
-	free(buffer.buffer);
+	strbuf_release(&buffer.buf);
 }
 
 static int remote_exists(const char *path)
@@ -2097,12 +2035,12 @@ static int remote_exists(const char *path)
 
 	sprintf(url, "%s%s", remote->url, path);
 
-        slot = get_active_slot();
+	slot = get_active_slot();
 	slot->results = &results;
-        curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-        curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
 
-        if (start_active_slot(slot)) {
+	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.http_code == 404)
 			return 0;
@@ -2120,17 +2058,13 @@ static int remote_exists(const char *path)
 static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 {
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	struct active_request_slot *slot;
 	struct slot_results results;
 
 	url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	sprintf(url, "%s%s", remote->url, path);
 
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = xmalloc(buffer.size);
-
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
@@ -2153,17 +2087,17 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 	*symref = NULL;
 	hashclr(sha1);
 
-	if (buffer.posn == 0)
+	if (buffer.len == 0)
 		return;
 
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
-	if (!prefixcmp((char *)buffer.buffer, "ref: ")) {
-		*symref = xmemdupz((char *)buffer.buffer + 5, buffer.posn - 6);
+	if (!prefixcmp((char *)buffer.buf, "ref: ")) {
+		*symref = xmemdupz((char *)buffer.buf + 5, buffer.len - 6);
 	} else {
-		get_sha1_hex(buffer.buffer, sha1);
+		get_sha1_hex(buffer.buf, sha1);
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 }
 
 static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)
diff --git a/http-walker.c b/http-walker.c
index a3fb596..d925005 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -48,7 +48,7 @@ struct alternates_request {
 	struct walker *walker;
 	const char *base;
 	char *url;
-	struct buffer *buffer;
+	struct strbuf *buffer;
 	struct active_request_slot *slot;
 	int http_specific;
 };
@@ -475,7 +475,7 @@ static void process_alternates_response(void *callback_data)
 
 	if (alt_req->http_specific) {
 		if (slot->curl_result != CURLE_OK ||
-		    !alt_req->buffer->posn) {
+		    !alt_req->buffer->len) {
 
 			/* Try reusing the slot to get non-http alternates */
 			alt_req->http_specific = 0;
@@ -503,12 +503,12 @@ static void process_alternates_response(void *callback_data)
 	}
 
 	fwrite_buffer(&null_byte, 1, 1, alt_req->buffer);
-	alt_req->buffer->posn--;
-	data = alt_req->buffer->buffer;
+	alt_req->buffer->len--;
+	data = alt_req->buffer->buf;
 
-	while (i < alt_req->buffer->posn) {
+	while (i < alt_req->buffer->len) {
 		int posn = i;
-		while (posn < alt_req->buffer->posn && data[posn] != '\n')
+		while (posn < alt_req->buffer->len && data[posn] != '\n')
 			posn++;
 		if (data[posn] == '\n') {
 			int okay = 0;
@@ -596,9 +596,8 @@ static void process_alternates_response(void *callback_data)
 
 static void fetch_alternates(struct walker *walker, const char *base)
 {
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *url;
-	char *data;
 	struct active_request_slot *slot;
 	struct alternates_request alt_req;
 	struct walker_data *cdata = walker->data;
@@ -619,11 +618,6 @@ static void fetch_alternates(struct walker *walker, const char *base)
 	/* Start the fetch */
 	cdata->got_alternates = 0;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (walker->get_verbosely)
 		fprintf(stderr, "Getting alternates list for %s\n", base);
 
@@ -652,7 +646,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
 	else
 		cdata->got_alternates = -1;
 
-	free(data);
+	strbuf_release(&buffer);
 	free(url);
 }
 
@@ -660,7 +654,7 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 {
 	unsigned char sha1[20];
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
 
@@ -670,11 +664,6 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	if (repo->got_indices)
 		return 0;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (walker->get_verbosely)
 		fprintf(stderr, "Getting pack list for %s\n", repo->base);
 
@@ -690,28 +679,27 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
+			strbuf_release(&buffer);
 			if (missing_target(&results)) {
 				repo->got_indices = 1;
-				free(buffer.buffer);
 				return 0;
 			} else {
 				repo->got_indices = 0;
-				free(buffer.buffer);
 				return error("%s", curl_errorstr);
 			}
 		}
 	} else {
 		repo->got_indices = 0;
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		return error("Unable to start request");
 	}
 
-	data = buffer.buffer;
-	while (i < buffer.posn) {
+	data = buffer.buf;
+	while (i < buffer.len) {
 		switch (data[i]) {
 		case 'P':
 			i++;
-			if (i + 52 <= buffer.posn &&
+			if (i + 52 <= buffer.len &&
 			    !prefixcmp(data + i, " pack-") &&
 			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
@@ -720,13 +708,13 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 				break;
 			}
 		default:
-			while (i < buffer.posn && data[i] != '\n')
+			while (i < buffer.len && data[i] != '\n')
 				i++;
 		}
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 	repo->got_indices = 1;
 	return 0;
 }
@@ -958,17 +946,12 @@ static char *quote_ref_url(const char *base, const char *ref)
 
 static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 {
-        char *url;
-        char hex[42];
-        struct buffer buffer;
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
 	struct walker_data *data = walker->data;
 	const char *base = data->alt->base;
 	struct active_request_slot *slot;
 	struct slot_results results;
-        buffer.size = 41;
-        buffer.posn = 0;
-        buffer.buffer = hex;
-        hex[41] = '\0';
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -986,9 +969,9 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
-        hex[40] = '\0';
-        get_sha1_hex(hex, sha1);
-        return 0;
+	buffer.buf[40] = '\0';
+	get_sha1_hex(buffer.buf, sha1);
+	return 0;
 }
 
 static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 146f626..2d0b46d 100644
--- a/http.c
+++ b/http.c
@@ -34,31 +34,25 @@ size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer)
 {
 	size_t size = eltsize * nmemb;
-	if (size > buffer->size - buffer->posn)
-		size = buffer->size - buffer->posn;
-	memcpy(ptr, (char *) buffer->buffer + buffer->posn, size);
+	if (size > buffer->buf.len - buffer->posn)
+		size = buffer->buf.len - buffer->posn;
+	memcpy(ptr, (char *) buffer->buf.buf + buffer->posn, size);
 	buffer->posn += size;
+
 	return size;
 }
 
 size_t fwrite_buffer(const void *ptr, size_t eltsize,
-			    size_t nmemb, struct buffer *buffer)
+			    size_t nmemb, struct strbuf *buffer)
 {
 	size_t size = eltsize * nmemb;
-	if (size > buffer->size - buffer->posn) {
-		buffer->size = buffer->size * 3 / 2;
-		if (buffer->size < buffer->posn + size)
-			buffer->size = buffer->posn + size;
-		buffer->buffer = xrealloc(buffer->buffer, buffer->size);
-	}
-	memcpy((char *) buffer->buffer + buffer->posn, ptr, size);
-	buffer->posn += size;
+	strbuf_add(buffer, ptr, size);
 	data_received++;
 	return size;
 }
 
 size_t fwrite_null(const void *ptr, size_t eltsize,
-			  size_t nmemb, struct buffer *buffer)
+			  size_t nmemb, struct strbuf *buffer)
 {
 	data_received++;
 	return eltsize * nmemb;
@@ -508,8 +502,8 @@ void run_active_slot(struct active_request_slot *slot)
 
 static void closedown_active_slot(struct active_request_slot *slot)
 {
-        active_requests--;
-        slot->in_use = 0;
+	active_requests--;
+	slot->in_use = 0;
 }
 
 void release_active_slot(struct active_request_slot *slot)
@@ -530,7 +524,7 @@ void release_active_slot(struct active_request_slot *slot)
 static void finish_active_slot(struct active_request_slot *slot)
 {
 	closedown_active_slot(slot);
-        curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
+	curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
 
 	if (slot->finished != NULL)
 		(*slot->finished) = 1;
@@ -541,10 +535,10 @@ static void finish_active_slot(struct active_request_slot *slot)
 		slot->results->http_code = slot->http_code;
 	}
 
-        /* Run callback if appropriate */
-        if (slot->callback_func != NULL) {
-                slot->callback_func(slot->callback_data);
-        }
+	/* Run callback if appropriate */
+	if (slot->callback_func != NULL) {
+		slot->callback_func(slot->callback_data);
+	}
 }
 
 void finish_all_active_slots(void)
diff --git a/http.h b/http.h
index fe1b0d1..bf3f12c 100644
--- a/http.h
+++ b/http.h
@@ -6,6 +6,8 @@
 #include <curl/curl.h>
 #include <curl/easy.h>
 
+#include "strbuf.h"
+
 #if LIBCURL_VERSION_NUM >= 0x071000
 #define USE_CURL_MULTI
 #define DEFAULT_MAX_REQUESTS 5
@@ -48,18 +50,17 @@ struct active_request_slot
 
 struct buffer
 {
-        size_t posn;
-        size_t size;
-        void *buffer;
+	struct strbuf buf;
+	size_t posn;
 };
 
 /* Curl request read/write callbacks */
 extern size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer);
 extern size_t fwrite_buffer(const void *ptr, size_t eltsize,
-			    size_t nmemb, struct buffer *buffer);
+			    size_t nmemb, struct strbuf *buffer);
 extern size_t fwrite_null(const void *ptr, size_t eltsize,
-			  size_t nmemb, struct buffer *buffer);
+			  size_t nmemb, struct strbuf *buffer);
 
 /* Slot lifecycle functions */
 extern struct active_request_slot *get_active_slot(void);
diff --git a/transport.c b/transport.c
index 58e66f6..22234e8 100644
--- a/transport.c
+++ b/transport.c
@@ -441,7 +441,7 @@ static int missing__target(int code, int result)
 
 static struct ref *get_refs_via_curl(struct transport *transport)
 {
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data, *start, *mid;
 	char *ref_name;
 	char *refs_url;
@@ -454,11 +454,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	struct ref *ref = NULL;
 	struct ref *last_ref = NULL;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	refs_url = xmalloc(strlen(transport->url) + 11);
 	sprintf(refs_url, "%s/info/refs", transport->url);
 
@@ -477,27 +472,26 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
+			strbuf_release(&buffer);
 			if (missing_target(&results)) {
-				free(buffer.buffer);
 				return NULL;
 			} else {
-				free(buffer.buffer);
 				error("%s", curl_errorstr);
 				return NULL;
 			}
 		}
 	} else {
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		error("Unable to start request");
 		return NULL;
 	}
 
 	http_cleanup();
 
-	data = buffer.buffer;
+	data = buffer.buf;
 	start = NULL;
 	mid = data;
-	while (i < buffer.posn) {
+	while (i < buffer.len) {
 		if (!start)
 			start = &data[i];
 		if (data[i] == '\t')
@@ -520,7 +514,7 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 
 	return refs;
 }
-- 
1.5.3.7

[PATCH 3/4] Move the file read logic to read_patch_file() in builtin-apply.c

[Mike Hommey]

This will allow to extend the read logic further. We also return a better
error message than usage() when the given filename can't be opened, and
avoid whitespace options not being set when reading from stdin with the
"-" argument as a side effect.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 builtin-apply.c |   32 +++++++++++++++++---------------
 1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index f2e9a33..8c8162a 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -182,11 +182,23 @@ static void say_patch_name(FILE *output, const char *pre,
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 
-static void read_patch_file(struct strbuf *sb, int fd)
+static void read_patch_file(struct strbuf *sb, const char *filename)
 {
+	int fd;
+
+	if (!strcmp(filename, "-")) {
+		fd = 0;
+	} else {
+		fd = open(filename, O_RDONLY);
+		if (fd < 0)
+			die("git-apply: could not open %s: %s", filename,
+			    strerror(errno));
+	}
+
 	if (strbuf_read(sb, fd, 0) < 0)
 		die("git-apply: read returned %s", strerror(errno));
 
+	close(fd);
 	/*
 	 * Make sure that we have some slop in the buffer
 	 * so that we can do speculative "memcmp" etc, and
@@ -2705,7 +2717,7 @@ static void prefix_patches(struct patch *p)
 	}
 }
 
-static int apply_patch(int fd, const char *filename, int inaccurate_eof)
+static int apply_patch(const char *filename, int inaccurate_eof)
 {
 	size_t offset;
 	struct strbuf buf;
@@ -2714,7 +2726,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)
 
 	strbuf_init(&buf, 0);
 	patch_input_file = filename;
-	read_patch_file(&buf, fd);
+	read_patch_file(&buf, filename);
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
@@ -2807,13 +2819,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		char *end;
-		int fd;
 
-		if (!strcmp(arg, "-")) {
-			errs |= apply_patch(0, "<stdin>", inaccurate_eof);
-			read_stdin = 0;
-			continue;
-		}
 		if (!prefixcmp(arg, "--exclude=")) {
 			struct excludes *x = xmalloc(sizeof(*x));
 			x->path = arg + 10;
@@ -2916,17 +2922,13 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		if (0 < prefix_length)
 			arg = prefix_filename(prefix, prefix_length, arg);
 
-		fd = open(arg, O_RDONLY);
-		if (fd < 0)
-			usage(apply_usage);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		errs |= apply_patch(fd, arg, inaccurate_eof);
-		close(fd);
+		errs |= apply_patch(arg, inaccurate_eof);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		errs |= apply_patch(0, "<stdin>", inaccurate_eof);
+		errs |= apply_patch("-", inaccurate_eof);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {
-- 
1.5.3.7

[PATCH 4/4] Add support for URLs to git-apply

[Mike Hommey]

Instead of doing several "wget -O - url | git-apply -" in a raw, you now
can just git-apply url1 url2 ...

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 Documentation/git-apply.txt |    3 +-
 builtin-apply.c             |   58 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index bae3e7b..2d5d725 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -25,7 +25,8 @@ OPTIONS
 -------
 <patch>...::
 	The files to read patch from.  '-' can be used to read
-	from the standard input.
+	from the standard input. They can also be http, https or
+	ftp URLs.
 
 --stat::
 	Instead of applying the patch, output diffstat for the
diff --git a/builtin-apply.c b/builtin-apply.c
index 8c8162a..59834f0 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -12,6 +12,9 @@
 #include "blob.h"
 #include "delta.h"
 #include "builtin.h"
+#ifndef NO_CURL
+#include "http.h"
+#endif
 
 /*
  *  --check turns on checking that the working tree matches the
@@ -182,12 +185,47 @@ static void say_patch_name(FILE *output, const char *pre,
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 
-static void read_patch_file(struct strbuf *sb, const char *filename)
+#ifndef NO_CURL
+static int used_http;
+
+static void read_patch_url(struct strbuf *sb, const char *url)
+{
+	struct active_request_slot *slot;
+	struct slot_results results;
+
+	if (! used_http) {
+		http_init();
+		used_http = 1;
+	}
+
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_FILE, sb);
+	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
+
+	if (start_active_slot(slot)) {
+		run_active_slot(slot);
+		if (results.curl_result != CURLE_OK)
+			die("git-apply: could not open %s", url);
+	}
+}
+#endif
+
+static void read_patch(struct strbuf *sb, const char *filename)
 {
 	int fd;
 
 	if (!strcmp(filename, "-")) {
 		fd = 0;
+#ifndef NO_CURL
+	} else if (!strncmp(filename, "http://", 7) ||
+		   !strncmp(filename, "https://", 8) ||
+		   !strncmp(filename, "ftp://", 6)) {
+		read_patch_url(sb, filename);
+		return;
+#endif
 	} else {
 		fd = open(filename, O_RDONLY);
 		if (fd < 0)
@@ -199,13 +237,6 @@ static void read_patch_file(struct strbuf *sb, const char *filename)
 		die("git-apply: read returned %s", strerror(errno));
 
 	close(fd);
-	/*
-	 * Make sure that we have some slop in the buffer
-	 * so that we can do speculative "memcmp" etc, and
-	 * see to it that it is NUL-filled.
-	 */
-	strbuf_grow(sb, SLOP);
-	memset(sb->buf + sb->len, 0, SLOP);
 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)
@@ -2726,7 +2757,14 @@ static int apply_patch(const char *filename, int inaccurate_eof)
 
 	strbuf_init(&buf, 0);
 	patch_input_file = filename;
-	read_patch_file(&buf, filename);
+	read_patch(&buf, filename);
+	/*
+	 * Make sure that we have some slop in the buffer
+	 * so that we can do speculative "memcmp" etc, and
+	 * see to it that it is NUL-filled.
+	 */
+	strbuf_grow(&buf, SLOP);
+	memset(buf.buf + buf.len, 0, SLOP);
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
@@ -2926,6 +2964,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		set_default_whitespace_mode(whitespace_option);
 		errs |= apply_patch(arg, inaccurate_eof);
 	}
+	if (used_http)
+		http_cleanup();
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
 		errs |= apply_patch("-", inaccurate_eof);
-- 
1.5.3.7

[Replacement PATCH 4/4] Add support for URLs to git-apply

[Mike Hommey]

Instead of doing several "wget -O - url | git-apply -" in a raw, you now
can just git-apply url1 url2 ...

Signed-off-by: Mike Hommey <mh@glandium.org>
---

 A #ifndef NO_CURL was missing in the previous patch.

 Documentation/git-apply.txt |    3 +-
 builtin-apply.c             |   60 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index bae3e7b..2d5d725 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -25,7 +25,8 @@ OPTIONS
 -------
 <patch>...::
 	The files to read patch from.  '-' can be used to read
-	from the standard input.
+	from the standard input. They can also be http, https or
+	ftp URLs.
 
 --stat::
 	Instead of applying the patch, output diffstat for the
diff --git a/builtin-apply.c b/builtin-apply.c
index 8c8162a..28645c7 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -12,6 +12,9 @@
 #include "blob.h"
 #include "delta.h"
 #include "builtin.h"
+#ifndef NO_CURL
+#include "http.h"
+#endif
 
 /*
  *  --check turns on checking that the working tree matches the
@@ -182,12 +185,47 @@ static void say_patch_name(FILE *output, const char *pre,
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 
-static void read_patch_file(struct strbuf *sb, const char *filename)
+#ifndef NO_CURL
+static int used_http;
+
+static void read_patch_url(struct strbuf *sb, const char *url)
+{
+	struct active_request_slot *slot;
+	struct slot_results results;
+
+	if (! used_http) {
+		http_init();
+		used_http = 1;
+	}
+
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_FILE, sb);
+	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
+
+	if (start_active_slot(slot)) {
+		run_active_slot(slot);
+		if (results.curl_result != CURLE_OK)
+			die("git-apply: could not open %s", url);
+	}
+}
+#endif
+
+static void read_patch(struct strbuf *sb, const char *filename)
 {
 	int fd;
 
 	if (!strcmp(filename, "-")) {
 		fd = 0;
+#ifndef NO_CURL
+	} else if (!strncmp(filename, "http://", 7) ||
+		   !strncmp(filename, "https://", 8) ||
+		   !strncmp(filename, "ftp://", 6)) {
+		read_patch_url(sb, filename);
+		return;
+#endif
 	} else {
 		fd = open(filename, O_RDONLY);
 		if (fd < 0)
@@ -199,13 +237,6 @@ static void read_patch_file(struct strbuf *sb, const char *filename)
 		die("git-apply: read returned %s", strerror(errno));
 
 	close(fd);
-	/*
-	 * Make sure that we have some slop in the buffer
-	 * so that we can do speculative "memcmp" etc, and
-	 * see to it that it is NUL-filled.
-	 */
-	strbuf_grow(sb, SLOP);
-	memset(sb->buf + sb->len, 0, SLOP);
 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)
@@ -2726,7 +2757,14 @@ static int apply_patch(const char *filename, int inaccurate_eof)
 
 	strbuf_init(&buf, 0);
 	patch_input_file = filename;
-	read_patch_file(&buf, filename);
+	read_patch(&buf, filename);
+	/*
+	 * Make sure that we have some slop in the buffer
+	 * so that we can do speculative "memcmp" etc, and
+	 * see to it that it is NUL-filled.
+	 */
+	strbuf_grow(&buf, SLOP);
+	memset(buf.buf + buf.len, 0, SLOP);
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
@@ -2926,6 +2964,10 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		set_default_whitespace_mode(whitespace_option);
 		errs |= apply_patch(arg, inaccurate_eof);
 	}
+#ifndef NO_CURL
+	if (used_http)
+		http_cleanup();
+#endif
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
 		errs |= apply_patch("-", inaccurate_eof);
-- 
1.5.3.7.1148.gcfe7

Re: [PATCH 3/4] Move the file read logic to read_patch_file() in builtin-apply.c

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> This will allow to extend the read logic further. We also return a better
> error message than usage() when the given filename can't be opened, and
> avoid whitespace options not being set when reading from stdin with the
> "-" argument as a side effect.

I think this is a good clean-up, but I suspect that apply_patch() should
be further refactored to take strbuf, instead of <fd, filename>
(original) or <filename> (yours).

Re: [PATCH 2/4] Use strbuf in http code

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> Also, replace whitespaces with tabs in some places
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
>  While testing this, I noticed 3 things:
>  - CURL_MULTI makes the code very racy
>  - a lot of the code doesn't do anything useful without CURL_MULTI
>  - the code is redundant

Yeah, there does seem to be a lot of duplicated code that does common
setup with slightly different request string.

> @@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
>  
>  int fetch_ref(char *ref, unsigned char *sha1)
>  {
> -        char *url;
> -        char hex[42];
> -        struct buffer buffer;
> +	char *url;
> +	struct strbuf buffer = STRBUF_INIT;
>  	char *base = remote->url;
>  	struct active_request_slot *slot;
>  	struct slot_results results;
> -        buffer.size = 41;
> -        buffer.posn = 0;
> -        buffer.buffer = hex;
> -        hex[41] = '\0';
>  
>  	url = quote_ref_url(base, ref);
>  	slot = get_active_slot();
> @@ -1142,9 +1131,9 @@ int fetch_ref(char *ref, unsigned char *sha1)
>  		return error("Unable to start request");
>  	}
>  
> -        hex[40] = '\0';
> -        get_sha1_hex(hex, sha1);
> -        return 0;
> +	buffer.buf[40] = '\0';
> +	get_sha1_hex(buffer.buf, sha1);
> +	return 0;
>  }

A conversion like this is worrysome and needs to be rethought I think.

At least with the old code, we knew hex[40] was a safe location to make
assignment to, even though we did not check if what it contained made
sense --- the other end might have had a garbage in that URL (but the
caller hopefully would be responsible for noticing that).  But with your
change, I do not think you have that guarantee.  fwrite_buffer() may
have extended the buffer using strbuf API, but it may have received less
than what you are expecting, in which case you may not have buf[40]
touchable for you, no?

I at the same time think the original code is buggy.  It initializes
buffer.buffer to the on-stack storage hex[], but lets fwrite_buffer() to
call xrealloc() on it.

Re: [PATCH 2/4] Use strbuf in http code

[Mike Hommey]

On Sun, Dec 09, 2007 at 10:15:15AM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > Also, replace whitespaces with tabs in some places
> >
> > Signed-off-by: Mike Hommey <mh@glandium.org>
> > ---
> >
> >  While testing this, I noticed 3 things:
> >  - CURL_MULTI makes the code very racy
> >  - a lot of the code doesn't do anything useful without CURL_MULTI
> >  - the code is redundant
> 
> Yeah, there does seem to be a lot of duplicated code that does common
> setup with slightly different request string.
> 
> > @@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
> >  
> >  int fetch_ref(char *ref, unsigned char *sha1)
> >  {
> > -        char *url;
> > -        char hex[42];
> > -        struct buffer buffer;
> > +	char *url;
> > +	struct strbuf buffer = STRBUF_INIT;
> >  	char *base = remote->url;
> >  	struct active_request_slot *slot;
> >  	struct slot_results results;
> > -        buffer.size = 41;
> > -        buffer.posn = 0;
> > -        buffer.buffer = hex;
> > -        hex[41] = '\0';
> >  
> >  	url = quote_ref_url(base, ref);
> >  	slot = get_active_slot();
> > @@ -1142,9 +1131,9 @@ int fetch_ref(char *ref, unsigned char *sha1)
> >  		return error("Unable to start request");
> >  	}
> >  
> > -        hex[40] = '\0';
> > -        get_sha1_hex(hex, sha1);
> > -        return 0;
> > +	buffer.buf[40] = '\0';
> > +	get_sha1_hex(buffer.buf, sha1);
> > +	return 0;
> >  }
> 
> A conversion like this is worrysome and needs to be rethought I think.
> 
> At least with the old code, we knew hex[40] was a safe location to make
> assignment to, even though we did not check if what it contained made
> sense --- the other end might have had a garbage in that URL (but the
> caller hopefully would be responsible for noticing that).  But with your
> change, I do not think you have that guarantee.  fwrite_buffer() may
> have extended the buffer using strbuf API, but it may have received less
> than what you are expecting, in which case you may not have buf[40]
> touchable for you, no?
> 
> I at the same time think the original code is buggy.  It initializes
> buffer.buffer to the on-stack storage hex[], but lets fwrite_buffer() to
> call xrealloc() on it.

Both codes are also buggy in case the ref is a symbolic ref, and that
happens. I got bitten by this while testing.

Considering the assumption being made that refs are all properly filled
with sha1s, both codes are mostly equally bad.

Fixing the issue would obviously be the subject for another patch.

Mike

Re: [PATCH 2/4] Use strbuf in http code

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> Both codes are also buggy in case the ref is a symbolic ref, and that
> happens. I got bitten by this while testing.
>
> Considering the assumption being made that refs are all properly filled
> with sha1s, both codes are mostly equally bad.
>
> Fixing the issue would obviously be the subject for another patch.

Ok, I'll reject [2/4] and expect a reroll.

[Resend PATCH 2/4] Use strbuf in http code

[Mike Hommey]

Also, replace whitespaces with tabs in some places

Signed-off-by: Mike Hommey <mh@glandium.org>
---

 While this doesn't fix the problem with symbolic refs, it does fail more
 cleanly.

 http-push.c   |  187 +++++++++++++++++++--------------------------------------
 http-walker.c |   60 +++++++------------
 http.c        |   34 ++++------
 http.h        |   11 ++--
 transport.c   |   18 ++----
 5 files changed, 109 insertions(+), 201 deletions(-)

diff --git a/http-push.c b/http-push.c
index 78283b4..a69d0e3 100644
--- a/http-push.c
+++ b/http-push.c
@@ -495,10 +495,10 @@ static void start_put(struct transfer_request *request)
 	memset(&stream, 0, sizeof(stream));
 	deflateInit(&stream, zlib_compression_level);
 	size = deflateBound(&stream, len + hdrlen);
-	request->buffer.buffer = xmalloc(size);
+	strbuf_init(&request->buffer.buf, size);
 
 	/* Compress it */
-	stream.next_out = request->buffer.buffer;
+	stream.next_out = (unsigned char *)request->buffer.buf.buf;
 	stream.avail_out = size;
 
 	/* First header.. */
@@ -515,8 +515,7 @@ static void start_put(struct transfer_request *request)
 	deflateEnd(&stream);
 	free(unpacked);
 
-	request->buffer.size = stream.total_out;
-	request->buffer.posn = 0;
+	request->buffer.buf.len = stream.total_out;
 
 	request->url = xmalloc(strlen(remote->url) +
 			       strlen(request->lock->token) + 51);
@@ -538,7 +537,7 @@ static void start_put(struct transfer_request *request)
 	slot->callback_func = process_response;
 	slot->callback_data = request;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &request->buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1003,18 +1002,13 @@ static int fetch_indices(void)
 {
 	unsigned char sha1[20];
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
 
 	struct active_request_slot *slot;
 	struct slot_results results;
 
-	data = xcalloc(1, 4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (push_verbosely)
 		fprintf(stderr, "Getting pack list\n");
 
@@ -1030,7 +1024,7 @@ static int fetch_indices(void)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			free(buffer.buffer);
+			strbuf_release(&buffer);
 			free(url);
 			if (results.http_code == 404)
 				return 0;
@@ -1038,18 +1032,18 @@ static int fetch_indices(void)
 				return error("%s", curl_errorstr);
 		}
 	} else {
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		free(url);
 		return error("Unable to start request");
 	}
 	free(url);
 
-	data = buffer.buffer;
-	while (i < buffer.posn) {
+	data = buffer.buf;
+	while (i < buffer.len) {
 		switch (data[i]) {
 		case 'P':
 			i++;
-			if (i + 52 < buffer.posn &&
+			if (i + 52 < buffer.len &&
 			    !prefixcmp(data + i, " pack-") &&
 			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
@@ -1064,7 +1058,7 @@ static int fetch_indices(void)
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 	return 0;
 }
 
@@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
 
 int fetch_ref(char *ref, unsigned char *sha1)
 {
-        char *url;
-        char hex[42];
-        struct buffer buffer;
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
 	char *base = remote->url;
 	struct active_request_slot *slot;
 	struct slot_results results;
-        buffer.size = 41;
-        buffer.posn = 0;
-        buffer.buffer = hex;
-        hex[41] = '\0';
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -1142,9 +1131,10 @@ int fetch_ref(char *ref, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
-        hex[40] = '\0';
-        get_sha1_hex(hex, sha1);
-        return 0;
+	strbuf_rtrim(&buffer);
+	if (buffer.len != 40)
+		return 1;
+	return get_sha1_hex(buffer.buf, sha1);
 }
 
 static void one_remote_object(const char *hex)
@@ -1267,10 +1257,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer out_buffer;
-	struct buffer in_buffer;
-	char *out_data;
-	char *in_data;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
+	struct strbuf in_buffer = STRBUF_INIT;
 	char *url;
 	char *ep;
 	char timeout_header[25];
@@ -1312,16 +1300,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 		ep = strchr(ep + 1, '/');
 	}
 
-	out_buffer.size = strlen(LOCK_REQUEST) + strlen(git_default_email) - 2;
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1, LOCK_REQUEST, git_default_email);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email);
 
 	sprintf(timeout_header, "Timeout: Second-%ld", timeout);
 	dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1330,7 +1309,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1354,8 +1333,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
 			XML_SetCharacterDataHandler(parser, xml_cdata);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 			if (result != XML_STATUS_OK) {
 				fprintf(stderr, "XML error: %s\n",
@@ -1369,8 +1348,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	}
 
 	curl_slist_free_all(dav_headers);
-	free(out_data);
-	free(in_data);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 
 	if (lock->token == NULL || lock->timeout <= 0) {
 		if (lock->token != NULL)
@@ -1521,10 +1500,8 @@ static void remote_ls(const char *path, int flags,
 	char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer in_buffer;
-	struct buffer out_buffer;
-	char *in_data;
-	char *out_data;
+	struct strbuf in_buffer = STRBUF_INIT;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	XML_Parser parser = XML_ParserCreate(NULL);
 	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
@@ -1540,16 +1517,7 @@ static void remote_ls(const char *path, int flags,
 
 	sprintf(url, "%s%s", remote->url, path);
 
-	out_buffer.size = strlen(PROPFIND_ALL_REQUEST);
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1, PROPFIND_ALL_REQUEST);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, PROPFIND_ALL_REQUEST);
 
 	dav_headers = curl_slist_append(dav_headers, "Depth: 1");
 	dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1557,7 +1525,7 @@ static void remote_ls(const char *path, int flags,
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1578,8 +1546,8 @@ static void remote_ls(const char *path, int flags,
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
 			XML_SetCharacterDataHandler(parser, xml_cdata);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 
 			if (result != XML_STATUS_OK) {
@@ -1594,8 +1562,8 @@ static void remote_ls(const char *path, int flags,
 
 	free(ls.path);
 	free(url);
-	free(out_data);
-	free(in_buffer.buffer);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 	curl_slist_free_all(dav_headers);
 }
 
@@ -1616,29 +1584,15 @@ static int locking_available(void)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer in_buffer;
-	struct buffer out_buffer;
-	char *in_data;
-	char *out_data;
+	struct strbuf in_buffer = STRBUF_INIT;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	XML_Parser parser = XML_ParserCreate(NULL);
 	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	int lock_flags = 0;
 
-	out_buffer.size =
-		strlen(PROPFIND_SUPPORTEDLOCK_REQUEST) +
-		strlen(remote->url) - 2;
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1,
-		 PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
 
 	dav_headers = curl_slist_append(dav_headers, "Depth: 0");
 	dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1646,7 +1600,7 @@ static int locking_available(void)
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1666,8 +1620,8 @@ static int locking_available(void)
 			XML_SetUserData(parser, &ctx);
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 
 			if (result != XML_STATUS_OK) {
@@ -1681,8 +1635,8 @@ static int locking_available(void)
 		fprintf(stderr, "Unable to start PROPFIND request\n");
 	}
 
-	free(out_data);
-	free(in_buffer.buffer);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 	curl_slist_free_all(dav_headers);
 
 	return lock_flags;
@@ -1800,30 +1754,20 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	char *out_data;
 	char *if_header;
-	struct buffer out_buffer;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	struct curl_slist *dav_headers = NULL;
-	int i;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
 	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 
-	out_buffer.size = 41;
-	out_data = xmalloc(out_buffer.size + 1);
-	i = snprintf(out_data, out_buffer.size + 1, "%s\n", sha1_to_hex(sha1));
-	if (i != out_buffer.size) {
-		fprintf(stderr, "Unable to initialize PUT request body\n");
-		return 0;
-	}
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
+	strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
 
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1834,7 +1778,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
-		free(out_data);
+		strbuf_release(&out_buffer.buf);
 		free(if_header);
 		if (results.curl_result != CURLE_OK) {
 			fprintf(stderr,
@@ -1844,7 +1788,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 			return 0;
 		}
 	} else {
-		free(out_data);
+		strbuf_release(&out_buffer.buf);
 		free(if_header);
 		fprintf(stderr, "Unable to start PUT request\n");
 		return 0;
@@ -2001,7 +1945,7 @@ static void mark_edges_uninteresting(struct commit_list *list)
 
 static void add_remote_info_ref(struct remote_ls_ctx *ls)
 {
-	struct buffer *buf = (struct buffer *)ls->userData;
+	struct strbuf *buf = (struct strbuf *)ls->userData;
 	unsigned char remote_sha1[20];
 	struct object *o;
 	int len;
@@ -2046,17 +1990,14 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
 
 static void update_remote_info_refs(struct remote_lock *lock)
 {
-	struct buffer buffer;
+	struct buffer buffer = { 0, STRBUF_INIT };
 	struct active_request_slot *slot;
 	struct slot_results results;
 	char *if_header;
 	struct curl_slist *dav_headers = NULL;
 
-	buffer.buffer = xcalloc(1, 4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
 	remote_ls("refs/", (PROCESS_FILES | RECURSIVE),
-		  add_remote_info_ref, &buffer);
+		  add_remote_info_ref, &buffer.buf);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
 		sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
@@ -2065,7 +2006,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		slot = get_active_slot();
 		slot->results = &results;
 		curl_easy_setopt(slot->curl, CURLOPT_INFILE, &buffer);
-		curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.posn);
+		curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.buf.len);
 		curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 		curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 		curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -2074,8 +2015,6 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
 		curl_easy_setopt(slot->curl, CURLOPT_URL, lock->url);
 
-		buffer.posn = 0;
-
 		if (start_active_slot(slot)) {
 			run_active_slot(slot);
 			if (results.curl_result != CURLE_OK) {
@@ -2086,7 +2025,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		}
 		free(if_header);
 	}
-	free(buffer.buffer);
+	strbuf_release(&buffer.buf);
 }
 
 static int remote_exists(const char *path)
@@ -2097,12 +2036,12 @@ static int remote_exists(const char *path)
 
 	sprintf(url, "%s%s", remote->url, path);
 
-        slot = get_active_slot();
+	slot = get_active_slot();
 	slot->results = &results;
-        curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-        curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
 
-        if (start_active_slot(slot)) {
+	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.http_code == 404)
 			return 0;
@@ -2120,17 +2059,13 @@ static int remote_exists(const char *path)
 static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 {
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	struct active_request_slot *slot;
 	struct slot_results results;
 
 	url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	sprintf(url, "%s%s", remote->url, path);
 
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = xmalloc(buffer.size);
-
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
@@ -2153,17 +2088,17 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 	*symref = NULL;
 	hashclr(sha1);
 
-	if (buffer.posn == 0)
+	if (buffer.len == 0)
 		return;
 
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
-	if (!prefixcmp((char *)buffer.buffer, "ref: ")) {
-		*symref = xmemdupz((char *)buffer.buffer + 5, buffer.posn - 6);
+	if (!prefixcmp((char *)buffer.buf, "ref: ")) {
+		*symref = xmemdupz((char *)buffer.buf + 5, buffer.len - 6);
 	} else {
-		get_sha1_hex(buffer.buffer, sha1);
+		get_sha1_hex(buffer.buf, sha1);
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 }
 
 static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)
diff --git a/http-walker.c b/http-walker.c
index a3fb596..d9a5f1e 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -48,7 +48,7 @@ struct alternates_request {
 	struct walker *walker;
 	const char *base;
 	char *url;
-	struct buffer *buffer;
+	struct strbuf *buffer;
 	struct active_request_slot *slot;
 	int http_specific;
 };
@@ -475,7 +475,7 @@ static void process_alternates_response(void *callback_data)
 
 	if (alt_req->http_specific) {
 		if (slot->curl_result != CURLE_OK ||
-		    !alt_req->buffer->posn) {
+		    !alt_req->buffer->len) {
 
 			/* Try reusing the slot to get non-http alternates */
 			alt_req->http_specific = 0;
@@ -503,12 +503,12 @@ static void process_alternates_response(void *callback_data)
 	}
 
 	fwrite_buffer(&null_byte, 1, 1, alt_req->buffer);
-	alt_req->buffer->posn--;
-	data = alt_req->buffer->buffer;
+	alt_req->buffer->len--;
+	data = alt_req->buffer->buf;
 
-	while (i < alt_req->buffer->posn) {
+	while (i < alt_req->buffer->len) {
 		int posn = i;
-		while (posn < alt_req->buffer->posn && data[posn] != '\n')
+		while (posn < alt_req->buffer->len && data[posn] != '\n')
 			posn++;
 		if (data[posn] == '\n') {
 			int okay = 0;
@@ -596,9 +596,8 @@ static void process_alternates_response(void *callback_data)
 
 static void fetch_alternates(struct walker *walker, const char *base)
 {
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *url;
-	char *data;
 	struct active_request_slot *slot;
 	struct alternates_request alt_req;
 	struct walker_data *cdata = walker->data;
@@ -619,11 +618,6 @@ static void fetch_alternates(struct walker *walker, const char *base)
 	/* Start the fetch */
 	cdata->got_alternates = 0;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (walker->get_verbosely)
 		fprintf(stderr, "Getting alternates list for %s\n", base);
 
@@ -652,7 +646,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
 	else
 		cdata->got_alternates = -1;
 
-	free(data);
+	strbuf_release(&buffer);
 	free(url);
 }
 
@@ -660,7 +654,7 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 {
 	unsigned char sha1[20];
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
 
@@ -670,11 +664,6 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	if (repo->got_indices)
 		return 0;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (walker->get_verbosely)
 		fprintf(stderr, "Getting pack list for %s\n", repo->base);
 
@@ -690,28 +679,27 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
+			strbuf_release(&buffer);
 			if (missing_target(&results)) {
 				repo->got_indices = 1;
-				free(buffer.buffer);
 				return 0;
 			} else {
 				repo->got_indices = 0;
-				free(buffer.buffer);
 				return error("%s", curl_errorstr);
 			}
 		}
 	} else {
 		repo->got_indices = 0;
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		return error("Unable to start request");
 	}
 
-	data = buffer.buffer;
-	while (i < buffer.posn) {
+	data = buffer.buf;
+	while (i < buffer.len) {
 		switch (data[i]) {
 		case 'P':
 			i++;
-			if (i + 52 <= buffer.posn &&
+			if (i + 52 <= buffer.len &&
 			    !prefixcmp(data + i, " pack-") &&
 			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
@@ -720,13 +708,13 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 				break;
 			}
 		default:
-			while (i < buffer.posn && data[i] != '\n')
+			while (i < buffer.len && data[i] != '\n')
 				i++;
 		}
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 	repo->got_indices = 1;
 	return 0;
 }
@@ -958,17 +946,12 @@ static char *quote_ref_url(const char *base, const char *ref)
 
 static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 {
-        char *url;
-        char hex[42];
-        struct buffer buffer;
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
 	struct walker_data *data = walker->data;
 	const char *base = data->alt->base;
 	struct active_request_slot *slot;
 	struct slot_results results;
-        buffer.size = 41;
-        buffer.posn = 0;
-        buffer.buffer = hex;
-        hex[41] = '\0';
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -986,9 +969,10 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
-        hex[40] = '\0';
-        get_sha1_hex(hex, sha1);
-        return 0;
+	strbuf_rtrim(&buffer);
+	if (buffer.len != 40)
+		return 1;
+	return get_sha1_hex(buffer.buf, sha1);
 }
 
 static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 146f626..2d0b46d 100644
--- a/http.c
+++ b/http.c
@@ -34,31 +34,25 @@ size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer)
 {
 	size_t size = eltsize * nmemb;
-	if (size > buffer->size - buffer->posn)
-		size = buffer->size - buffer->posn;
-	memcpy(ptr, (char *) buffer->buffer + buffer->posn, size);
+	if (size > buffer->buf.len - buffer->posn)
+		size = buffer->buf.len - buffer->posn;
+	memcpy(ptr, (char *) buffer->buf.buf + buffer->posn, size);
 	buffer->posn += size;
+
 	return size;
 }
 
 size_t fwrite_buffer(const void *ptr, size_t eltsize,
-			    size_t nmemb, struct buffer *buffer)
+			    size_t nmemb, struct strbuf *buffer)
 {
 	size_t size = eltsize * nmemb;
-	if (size > buffer->size - buffer->posn) {
-		buffer->size = buffer->size * 3 / 2;
-		if (buffer->size < buffer->posn + size)
-			buffer->size = buffer->posn + size;
-		buffer->buffer = xrealloc(buffer->buffer, buffer->size);
-	}
-	memcpy((char *) buffer->buffer + buffer->posn, ptr, size);
-	buffer->posn += size;
+	strbuf_add(buffer, ptr, size);
 	data_received++;
 	return size;
 }
 
 size_t fwrite_null(const void *ptr, size_t eltsize,
-			  size_t nmemb, struct buffer *buffer)
+			  size_t nmemb, struct strbuf *buffer)
 {
 	data_received++;
 	return eltsize * nmemb;
@@ -508,8 +502,8 @@ void run_active_slot(struct active_request_slot *slot)
 
 static void closedown_active_slot(struct active_request_slot *slot)
 {
-        active_requests--;
-        slot->in_use = 0;
+	active_requests--;
+	slot->in_use = 0;
 }
 
 void release_active_slot(struct active_request_slot *slot)
@@ -530,7 +524,7 @@ void release_active_slot(struct active_request_slot *slot)
 static void finish_active_slot(struct active_request_slot *slot)
 {
 	closedown_active_slot(slot);
-        curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
+	curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
 
 	if (slot->finished != NULL)
 		(*slot->finished) = 1;
@@ -541,10 +535,10 @@ static void finish_active_slot(struct active_request_slot *slot)
 		slot->results->http_code = slot->http_code;
 	}
 
-        /* Run callback if appropriate */
-        if (slot->callback_func != NULL) {
-                slot->callback_func(slot->callback_data);
-        }
+	/* Run callback if appropriate */
+	if (slot->callback_func != NULL) {
+		slot->callback_func(slot->callback_data);
+	}
 }
 
 void finish_all_active_slots(void)
diff --git a/http.h b/http.h
index fe1b0d1..bf3f12c 100644
--- a/http.h
+++ b/http.h
@@ -6,6 +6,8 @@
 #include <curl/curl.h>
 #include <curl/easy.h>
 
+#include "strbuf.h"
+
 #if LIBCURL_VERSION_NUM >= 0x071000
 #define USE_CURL_MULTI
 #define DEFAULT_MAX_REQUESTS 5
@@ -48,18 +50,17 @@ struct active_request_slot
 
 struct buffer
 {
-        size_t posn;
-        size_t size;
-        void *buffer;
+	struct strbuf buf;
+	size_t posn;
 };
 
 /* Curl request read/write callbacks */
 extern size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer);
 extern size_t fwrite_buffer(const void *ptr, size_t eltsize,
-			    size_t nmemb, struct buffer *buffer);
+			    size_t nmemb, struct strbuf *buffer);
 extern size_t fwrite_null(const void *ptr, size_t eltsize,
-			  size_t nmemb, struct buffer *buffer);
+			  size_t nmemb, struct strbuf *buffer);
 
 /* Slot lifecycle functions */
 extern struct active_request_slot *get_active_slot(void);
diff --git a/transport.c b/transport.c
index 58e66f6..22234e8 100644
--- a/transport.c
+++ b/transport.c
@@ -441,7 +441,7 @@ static int missing__target(int code, int result)
 
 static struct ref *get_refs_via_curl(struct transport *transport)
 {
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data, *start, *mid;
 	char *ref_name;
 	char *refs_url;
@@ -454,11 +454,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	struct ref *ref = NULL;
 	struct ref *last_ref = NULL;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	refs_url = xmalloc(strlen(transport->url) + 11);
 	sprintf(refs_url, "%s/info/refs", transport->url);
 
@@ -477,27 +472,26 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
+			strbuf_release(&buffer);
 			if (missing_target(&results)) {
-				free(buffer.buffer);
 				return NULL;
 			} else {
-				free(buffer.buffer);
 				error("%s", curl_errorstr);
 				return NULL;
 			}
 		}
 	} else {
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		error("Unable to start request");
 		return NULL;
 	}
 
 	http_cleanup();
 
-	data = buffer.buffer;
+	data = buffer.buf;
 	start = NULL;
 	mid = data;
-	while (i < buffer.posn) {
+	while (i < buffer.len) {
 		if (!start)
 			start = &data[i];
 		if (data[i] == '\t')
@@ -520,7 +514,7 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 
 	return refs;
 }
-- 
1.5.3.7

Re: [Resend PATCH 2/4] Use strbuf in http code

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> Also, replace whitespaces with tabs in some places
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
>  While this doesn't fix the problem with symbolic refs, it does fail more
>  cleanly.
>
>  http-push.c   |  187 +++++++++++++++++++--------------------------------------
>  http-walker.c |   60 +++++++------------
>  http.c        |   34 ++++------
>  http.h        |   11 ++--
>  transport.c   |   18 ++----
>  5 files changed, 109 insertions(+), 201 deletions(-)

I like this code reduction, but

> diff --git a/http.h b/http.h
> index fe1b0d1..bf3f12c 100644
> --- a/http.h
> +++ b/http.h
> @@ -6,6 +6,8 @@
>  #include <curl/curl.h>
>  #include <curl/easy.h>
>  
> +#include "strbuf.h"
> +
>  #if LIBCURL_VERSION_NUM >= 0x071000
>  #define USE_CURL_MULTI
>  #define DEFAULT_MAX_REQUESTS 5
> @@ -48,18 +50,17 @@ struct active_request_slot
>  
>  struct buffer
>  {
> -        size_t posn;
> -        size_t size;
> -        void *buffer;
> +	struct strbuf buf;
> +	size_t posn;
>  };

With this definition of "struct buffer", I do not think this can be correct.

> @@ -1267,10 +1257,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
>  {
>  	struct active_request_slot *slot;
>  	struct slot_results results;
> -	struct buffer out_buffer;
> -	struct buffer in_buffer;
> -	char *out_data;
> -	char *in_data;
> +	struct buffer out_buffer = { 0, STRBUF_INIT };

How seriously have you proofread and tested this series before sending
it out?

Re: [Resend PATCH 2/4] Use strbuf in http code

[Mike Hommey]

On Mon, Dec 10, 2007 at 10:04:52PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> >  struct buffer
> >  {
> > -        size_t posn;
> > -        size_t size;
> > -        void *buffer;
> > +	struct strbuf buf;
> > +	size_t posn;
> >  };
> 
> With this definition of "struct buffer", I do not think this can be correct.
> 
> > @@ -1267,10 +1257,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
> >  {
> >  	struct active_request_slot *slot;
> >  	struct slot_results results;
> > -	struct buffer out_buffer;
> > -	struct buffer in_buffer;
> > -	char *out_data;
> > -	char *in_data;
> > +	struct buffer out_buffer = { 0, STRBUF_INIT };
> 
> How seriously have you proofread and tested this series before sending
> it out?

Damn ! I did fix this on sunday and was pretty sure to have sent it :-/

Mike

[Replacement PATCH 2/4] Use strbuf in http code

[Mike Hommey]

Also, replace whitespaces with tabs in some places

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 http-push.c   |  187 +++++++++++++++++++--------------------------------------
 http-walker.c |   60 +++++++------------
 http.c        |   34 ++++------
 http.h        |   11 ++--
 transport.c   |   18 ++----
 5 files changed, 109 insertions(+), 201 deletions(-)

diff --git a/http-push.c b/http-push.c
index 78283b4..a69d0e3 100644
--- a/http-push.c
+++ b/http-push.c
@@ -495,10 +495,10 @@ static void start_put(struct transfer_request *request)
 	memset(&stream, 0, sizeof(stream));
 	deflateInit(&stream, zlib_compression_level);
 	size = deflateBound(&stream, len + hdrlen);
-	request->buffer.buffer = xmalloc(size);
+	strbuf_init(&request->buffer.buf, size);
 
 	/* Compress it */
-	stream.next_out = request->buffer.buffer;
+	stream.next_out = (unsigned char *)request->buffer.buf.buf;
 	stream.avail_out = size;
 
 	/* First header.. */
@@ -515,8 +515,7 @@ static void start_put(struct transfer_request *request)
 	deflateEnd(&stream);
 	free(unpacked);
 
-	request->buffer.size = stream.total_out;
-	request->buffer.posn = 0;
+	request->buffer.buf.len = stream.total_out;
 
 	request->url = xmalloc(strlen(remote->url) +
 			       strlen(request->lock->token) + 51);
@@ -538,7 +537,7 @@ static void start_put(struct transfer_request *request)
 	slot->callback_func = process_response;
 	slot->callback_data = request;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &request->buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1003,18 +1002,13 @@ static int fetch_indices(void)
 {
 	unsigned char sha1[20];
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
 
 	struct active_request_slot *slot;
 	struct slot_results results;
 
-	data = xcalloc(1, 4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (push_verbosely)
 		fprintf(stderr, "Getting pack list\n");
 
@@ -1030,7 +1024,7 @@ static int fetch_indices(void)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			free(buffer.buffer);
+			strbuf_release(&buffer);
 			free(url);
 			if (results.http_code == 404)
 				return 0;
@@ -1038,18 +1032,18 @@ static int fetch_indices(void)
 				return error("%s", curl_errorstr);
 		}
 	} else {
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		free(url);
 		return error("Unable to start request");
 	}
 	free(url);
 
-	data = buffer.buffer;
-	while (i < buffer.posn) {
+	data = buffer.buf;
+	while (i < buffer.len) {
 		switch (data[i]) {
 		case 'P':
 			i++;
-			if (i + 52 < buffer.posn &&
+			if (i + 52 < buffer.len &&
 			    !prefixcmp(data + i, " pack-") &&
 			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
@@ -1064,7 +1058,7 @@ static int fetch_indices(void)
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 	return 0;
 }
 
@@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
 
 int fetch_ref(char *ref, unsigned char *sha1)
 {
-        char *url;
-        char hex[42];
-        struct buffer buffer;
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
 	char *base = remote->url;
 	struct active_request_slot *slot;
 	struct slot_results results;
-        buffer.size = 41;
-        buffer.posn = 0;
-        buffer.buffer = hex;
-        hex[41] = '\0';
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -1142,9 +1131,10 @@ int fetch_ref(char *ref, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
-        hex[40] = '\0';
-        get_sha1_hex(hex, sha1);
-        return 0;
+	strbuf_rtrim(&buffer);
+	if (buffer.len != 40)
+		return 1;
+	return get_sha1_hex(buffer.buf, sha1);
 }
 
 static void one_remote_object(const char *hex)
@@ -1267,10 +1257,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer out_buffer;
-	struct buffer in_buffer;
-	char *out_data;
-	char *in_data;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
+	struct strbuf in_buffer = STRBUF_INIT;
 	char *url;
 	char *ep;
 	char timeout_header[25];
@@ -1312,16 +1300,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 		ep = strchr(ep + 1, '/');
 	}
 
-	out_buffer.size = strlen(LOCK_REQUEST) + strlen(git_default_email) - 2;
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1, LOCK_REQUEST, git_default_email);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email);
 
 	sprintf(timeout_header, "Timeout: Second-%ld", timeout);
 	dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1330,7 +1309,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1354,8 +1333,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
 			XML_SetCharacterDataHandler(parser, xml_cdata);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 			if (result != XML_STATUS_OK) {
 				fprintf(stderr, "XML error: %s\n",
@@ -1369,8 +1348,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
 	}
 
 	curl_slist_free_all(dav_headers);
-	free(out_data);
-	free(in_data);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 
 	if (lock->token == NULL || lock->timeout <= 0) {
 		if (lock->token != NULL)
@@ -1521,10 +1500,8 @@ static void remote_ls(const char *path, int flags,
 	char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer in_buffer;
-	struct buffer out_buffer;
-	char *in_data;
-	char *out_data;
+	struct strbuf in_buffer = STRBUF_INIT;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	XML_Parser parser = XML_ParserCreate(NULL);
 	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
@@ -1540,16 +1517,7 @@ static void remote_ls(const char *path, int flags,
 
 	sprintf(url, "%s%s", remote->url, path);
 
-	out_buffer.size = strlen(PROPFIND_ALL_REQUEST);
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1, PROPFIND_ALL_REQUEST);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, PROPFIND_ALL_REQUEST);
 
 	dav_headers = curl_slist_append(dav_headers, "Depth: 1");
 	dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1557,7 +1525,7 @@ static void remote_ls(const char *path, int flags,
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1578,8 +1546,8 @@ static void remote_ls(const char *path, int flags,
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
 			XML_SetCharacterDataHandler(parser, xml_cdata);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 
 			if (result != XML_STATUS_OK) {
@@ -1594,8 +1562,8 @@ static void remote_ls(const char *path, int flags,
 
 	free(ls.path);
 	free(url);
-	free(out_data);
-	free(in_buffer.buffer);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 	curl_slist_free_all(dav_headers);
 }
 
@@ -1616,29 +1584,15 @@ static int locking_available(void)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	struct buffer in_buffer;
-	struct buffer out_buffer;
-	char *in_data;
-	char *out_data;
+	struct strbuf in_buffer = STRBUF_INIT;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	XML_Parser parser = XML_ParserCreate(NULL);
 	enum XML_Status result;
 	struct curl_slist *dav_headers = NULL;
 	struct xml_ctx ctx;
 	int lock_flags = 0;
 
-	out_buffer.size =
-		strlen(PROPFIND_SUPPORTEDLOCK_REQUEST) +
-		strlen(remote->url) - 2;
-	out_data = xmalloc(out_buffer.size + 1);
-	snprintf(out_data, out_buffer.size + 1,
-		 PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
-
-	in_buffer.size = 4096;
-	in_data = xmalloc(in_buffer.size);
-	in_buffer.posn = 0;
-	in_buffer.buffer = in_data;
+	strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
 
 	dav_headers = curl_slist_append(dav_headers, "Depth: 0");
 	dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1646,7 +1600,7 @@ static int locking_available(void)
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1666,8 +1620,8 @@ static int locking_available(void)
 			XML_SetUserData(parser, &ctx);
 			XML_SetElementHandler(parser, xml_start_tag,
 					      xml_end_tag);
-			result = XML_Parse(parser, in_buffer.buffer,
-					   in_buffer.posn, 1);
+			result = XML_Parse(parser, in_buffer.buf,
+					   in_buffer.len, 1);
 			free(ctx.name);
 
 			if (result != XML_STATUS_OK) {
@@ -1681,8 +1635,8 @@ static int locking_available(void)
 		fprintf(stderr, "Unable to start PROPFIND request\n");
 	}
 
-	free(out_data);
-	free(in_buffer.buffer);
+	strbuf_release(&out_buffer.buf);
+	strbuf_release(&in_buffer);
 	curl_slist_free_all(dav_headers);
 
 	return lock_flags;
@@ -1800,30 +1754,20 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 {
 	struct active_request_slot *slot;
 	struct slot_results results;
-	char *out_data;
 	char *if_header;
-	struct buffer out_buffer;
+	struct buffer out_buffer = { 0, STRBUF_INIT };
 	struct curl_slist *dav_headers = NULL;
-	int i;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
 	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 
-	out_buffer.size = 41;
-	out_data = xmalloc(out_buffer.size + 1);
-	i = snprintf(out_data, out_buffer.size + 1, "%s\n", sha1_to_hex(sha1));
-	if (i != out_buffer.size) {
-		fprintf(stderr, "Unable to initialize PUT request body\n");
-		return 0;
-	}
-	out_buffer.posn = 0;
-	out_buffer.buffer = out_data;
+	strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
 
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+	curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
 	curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1834,7 +1778,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
-		free(out_data);
+		strbuf_release(&out_buffer.buf);
 		free(if_header);
 		if (results.curl_result != CURLE_OK) {
 			fprintf(stderr,
@@ -1844,7 +1788,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 			return 0;
 		}
 	} else {
-		free(out_data);
+		strbuf_release(&out_buffer.buf);
 		free(if_header);
 		fprintf(stderr, "Unable to start PUT request\n");
 		return 0;
@@ -2001,7 +1945,7 @@ static void mark_edges_uninteresting(struct commit_list *list)
 
 static void add_remote_info_ref(struct remote_ls_ctx *ls)
 {
-	struct buffer *buf = (struct buffer *)ls->userData;
+	struct strbuf *buf = (struct strbuf *)ls->userData;
 	unsigned char remote_sha1[20];
 	struct object *o;
 	int len;
@@ -2046,17 +1990,14 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
 
 static void update_remote_info_refs(struct remote_lock *lock)
 {
-	struct buffer buffer;
+	struct buffer buffer = { 0, STRBUF_INIT };
 	struct active_request_slot *slot;
 	struct slot_results results;
 	char *if_header;
 	struct curl_slist *dav_headers = NULL;
 
-	buffer.buffer = xcalloc(1, 4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
 	remote_ls("refs/", (PROCESS_FILES | RECURSIVE),
-		  add_remote_info_ref, &buffer);
+		  add_remote_info_ref, &buffer.buf);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
 		sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
@@ -2065,7 +2006,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		slot = get_active_slot();
 		slot->results = &results;
 		curl_easy_setopt(slot->curl, CURLOPT_INFILE, &buffer);
-		curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.posn);
+		curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.buf.len);
 		curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
 		curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
 		curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -2074,8 +2015,6 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
 		curl_easy_setopt(slot->curl, CURLOPT_URL, lock->url);
 
-		buffer.posn = 0;
-
 		if (start_active_slot(slot)) {
 			run_active_slot(slot);
 			if (results.curl_result != CURLE_OK) {
@@ -2086,7 +2025,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		}
 		free(if_header);
 	}
-	free(buffer.buffer);
+	strbuf_release(&buffer.buf);
 }
 
 static int remote_exists(const char *path)
@@ -2097,12 +2036,12 @@ static int remote_exists(const char *path)
 
 	sprintf(url, "%s%s", remote->url, path);
 
-        slot = get_active_slot();
+	slot = get_active_slot();
 	slot->results = &results;
-        curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-        curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
 
-        if (start_active_slot(slot)) {
+	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.http_code == 404)
 			return 0;
@@ -2120,17 +2059,13 @@ static int remote_exists(const char *path)
 static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 {
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	struct active_request_slot *slot;
 	struct slot_results results;
 
 	url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	sprintf(url, "%s%s", remote->url, path);
 
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = xmalloc(buffer.size);
-
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
@@ -2153,17 +2088,17 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 	*symref = NULL;
 	hashclr(sha1);
 
-	if (buffer.posn == 0)
+	if (buffer.len == 0)
 		return;
 
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
-	if (!prefixcmp((char *)buffer.buffer, "ref: ")) {
-		*symref = xmemdupz((char *)buffer.buffer + 5, buffer.posn - 6);
+	if (!prefixcmp((char *)buffer.buf, "ref: ")) {
+		*symref = xmemdupz((char *)buffer.buf + 5, buffer.len - 6);
 	} else {
-		get_sha1_hex(buffer.buffer, sha1);
+		get_sha1_hex(buffer.buf, sha1);
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 }
 
 static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)
diff --git a/http-walker.c b/http-walker.c
index a3fb596..d9a5f1e 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -48,7 +48,7 @@ struct alternates_request {
 	struct walker *walker;
 	const char *base;
 	char *url;
-	struct buffer *buffer;
+	struct strbuf *buffer;
 	struct active_request_slot *slot;
 	int http_specific;
 };
@@ -475,7 +475,7 @@ static void process_alternates_response(void *callback_data)
 
 	if (alt_req->http_specific) {
 		if (slot->curl_result != CURLE_OK ||
-		    !alt_req->buffer->posn) {
+		    !alt_req->buffer->len) {
 
 			/* Try reusing the slot to get non-http alternates */
 			alt_req->http_specific = 0;
@@ -503,12 +503,12 @@ static void process_alternates_response(void *callback_data)
 	}
 
 	fwrite_buffer(&null_byte, 1, 1, alt_req->buffer);
-	alt_req->buffer->posn--;
-	data = alt_req->buffer->buffer;
+	alt_req->buffer->len--;
+	data = alt_req->buffer->buf;
 
-	while (i < alt_req->buffer->posn) {
+	while (i < alt_req->buffer->len) {
 		int posn = i;
-		while (posn < alt_req->buffer->posn && data[posn] != '\n')
+		while (posn < alt_req->buffer->len && data[posn] != '\n')
 			posn++;
 		if (data[posn] == '\n') {
 			int okay = 0;
@@ -596,9 +596,8 @@ static void process_alternates_response(void *callback_data)
 
 static void fetch_alternates(struct walker *walker, const char *base)
 {
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *url;
-	char *data;
 	struct active_request_slot *slot;
 	struct alternates_request alt_req;
 	struct walker_data *cdata = walker->data;
@@ -619,11 +618,6 @@ static void fetch_alternates(struct walker *walker, const char *base)
 	/* Start the fetch */
 	cdata->got_alternates = 0;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (walker->get_verbosely)
 		fprintf(stderr, "Getting alternates list for %s\n", base);
 
@@ -652,7 +646,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
 	else
 		cdata->got_alternates = -1;
 
-	free(data);
+	strbuf_release(&buffer);
 	free(url);
 }
 
@@ -660,7 +654,7 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 {
 	unsigned char sha1[20];
 	char *url;
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
 
@@ -670,11 +664,6 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	if (repo->got_indices)
 		return 0;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	if (walker->get_verbosely)
 		fprintf(stderr, "Getting pack list for %s\n", repo->base);
 
@@ -690,28 +679,27 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
+			strbuf_release(&buffer);
 			if (missing_target(&results)) {
 				repo->got_indices = 1;
-				free(buffer.buffer);
 				return 0;
 			} else {
 				repo->got_indices = 0;
-				free(buffer.buffer);
 				return error("%s", curl_errorstr);
 			}
 		}
 	} else {
 		repo->got_indices = 0;
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		return error("Unable to start request");
 	}
 
-	data = buffer.buffer;
-	while (i < buffer.posn) {
+	data = buffer.buf;
+	while (i < buffer.len) {
 		switch (data[i]) {
 		case 'P':
 			i++;
-			if (i + 52 <= buffer.posn &&
+			if (i + 52 <= buffer.len &&
 			    !prefixcmp(data + i, " pack-") &&
 			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
@@ -720,13 +708,13 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 				break;
 			}
 		default:
-			while (i < buffer.posn && data[i] != '\n')
+			while (i < buffer.len && data[i] != '\n')
 				i++;
 		}
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 	repo->got_indices = 1;
 	return 0;
 }
@@ -958,17 +946,12 @@ static char *quote_ref_url(const char *base, const char *ref)
 
 static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 {
-        char *url;
-        char hex[42];
-        struct buffer buffer;
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
 	struct walker_data *data = walker->data;
 	const char *base = data->alt->base;
 	struct active_request_slot *slot;
 	struct slot_results results;
-        buffer.size = 41;
-        buffer.posn = 0;
-        buffer.buffer = hex;
-        hex[41] = '\0';
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -986,9 +969,10 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
-        hex[40] = '\0';
-        get_sha1_hex(hex, sha1);
-        return 0;
+	strbuf_rtrim(&buffer);
+	if (buffer.len != 40)
+		return 1;
+	return get_sha1_hex(buffer.buf, sha1);
 }
 
 static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 146f626..2d0b46d 100644
--- a/http.c
+++ b/http.c
@@ -34,31 +34,25 @@ size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer)
 {
 	size_t size = eltsize * nmemb;
-	if (size > buffer->size - buffer->posn)
-		size = buffer->size - buffer->posn;
-	memcpy(ptr, (char *) buffer->buffer + buffer->posn, size);
+	if (size > buffer->buf.len - buffer->posn)
+		size = buffer->buf.len - buffer->posn;
+	memcpy(ptr, (char *) buffer->buf.buf + buffer->posn, size);
 	buffer->posn += size;
+
 	return size;
 }
 
 size_t fwrite_buffer(const void *ptr, size_t eltsize,
-			    size_t nmemb, struct buffer *buffer)
+			    size_t nmemb, struct strbuf *buffer)
 {
 	size_t size = eltsize * nmemb;
-	if (size > buffer->size - buffer->posn) {
-		buffer->size = buffer->size * 3 / 2;
-		if (buffer->size < buffer->posn + size)
-			buffer->size = buffer->posn + size;
-		buffer->buffer = xrealloc(buffer->buffer, buffer->size);
-	}
-	memcpy((char *) buffer->buffer + buffer->posn, ptr, size);
-	buffer->posn += size;
+	strbuf_add(buffer, ptr, size);
 	data_received++;
 	return size;
 }
 
 size_t fwrite_null(const void *ptr, size_t eltsize,
-			  size_t nmemb, struct buffer *buffer)
+			  size_t nmemb, struct strbuf *buffer)
 {
 	data_received++;
 	return eltsize * nmemb;
@@ -508,8 +502,8 @@ void run_active_slot(struct active_request_slot *slot)
 
 static void closedown_active_slot(struct active_request_slot *slot)
 {
-        active_requests--;
-        slot->in_use = 0;
+	active_requests--;
+	slot->in_use = 0;
 }
 
 void release_active_slot(struct active_request_slot *slot)
@@ -530,7 +524,7 @@ void release_active_slot(struct active_request_slot *slot)
 static void finish_active_slot(struct active_request_slot *slot)
 {
 	closedown_active_slot(slot);
-        curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
+	curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
 
 	if (slot->finished != NULL)
 		(*slot->finished) = 1;
@@ -541,10 +535,10 @@ static void finish_active_slot(struct active_request_slot *slot)
 		slot->results->http_code = slot->http_code;
 	}
 
-        /* Run callback if appropriate */
-        if (slot->callback_func != NULL) {
-                slot->callback_func(slot->callback_data);
-        }
+	/* Run callback if appropriate */
+	if (slot->callback_func != NULL) {
+		slot->callback_func(slot->callback_data);
+	}
 }
 
 void finish_all_active_slots(void)
diff --git a/http.h b/http.h
index fe1b0d1..a0fb4cf 100644
--- a/http.h
+++ b/http.h
@@ -6,6 +6,8 @@
 #include <curl/curl.h>
 #include <curl/easy.h>
 
+#include "strbuf.h"
+
 #if LIBCURL_VERSION_NUM >= 0x071000
 #define USE_CURL_MULTI
 #define DEFAULT_MAX_REQUESTS 5
@@ -48,18 +50,17 @@ struct active_request_slot
 
 struct buffer
 {
-        size_t posn;
-        size_t size;
-        void *buffer;
+	size_t posn;
+	struct strbuf buf;
 };
 
 /* Curl request read/write callbacks */
 extern size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
 			   struct buffer *buffer);
 extern size_t fwrite_buffer(const void *ptr, size_t eltsize,
-			    size_t nmemb, struct buffer *buffer);
+			    size_t nmemb, struct strbuf *buffer);
 extern size_t fwrite_null(const void *ptr, size_t eltsize,
-			  size_t nmemb, struct buffer *buffer);
+			  size_t nmemb, struct strbuf *buffer);
 
 /* Slot lifecycle functions */
 extern struct active_request_slot *get_active_slot(void);
diff --git a/transport.c b/transport.c
index 58e66f6..22234e8 100644
--- a/transport.c
+++ b/transport.c
@@ -441,7 +441,7 @@ static int missing__target(int code, int result)
 
 static struct ref *get_refs_via_curl(struct transport *transport)
 {
-	struct buffer buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	char *data, *start, *mid;
 	char *ref_name;
 	char *refs_url;
@@ -454,11 +454,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	struct ref *ref = NULL;
 	struct ref *last_ref = NULL;
 
-	data = xmalloc(4096);
-	buffer.size = 4096;
-	buffer.posn = 0;
-	buffer.buffer = data;
-
 	refs_url = xmalloc(strlen(transport->url) + 11);
 	sprintf(refs_url, "%s/info/refs", transport->url);
 
@@ -477,27 +472,26 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
+			strbuf_release(&buffer);
 			if (missing_target(&results)) {
-				free(buffer.buffer);
 				return NULL;
 			} else {
-				free(buffer.buffer);
 				error("%s", curl_errorstr);
 				return NULL;
 			}
 		}
 	} else {
-		free(buffer.buffer);
+		strbuf_release(&buffer);
 		error("Unable to start request");
 		return NULL;
 	}
 
 	http_cleanup();
 
-	data = buffer.buffer;
+	data = buffer.buf;
 	start = NULL;
 	mid = data;
-	while (i < buffer.posn) {
+	while (i < buffer.len) {
 		if (!start)
 			start = &data[i];
 		if (data[i] == '\t')
@@ -520,7 +514,7 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		i++;
 	}
 
-	free(buffer.buffer);
+	strbuf_release(&buffer);
 
 	return refs;
 }
-- 
1.5.3.7.1161.g14b8

[PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Mike Hommey]

3803bcea tried to fix this, but it only adds the branckes when the given
In-Reply-To begins and ends with whitespaces. It also didn't do anything
to the --in-reply-to argument.

Signed-off-by: Mike Hommey <mh@glandium.org>
---

I just got bitten by this...

 git-send-email.perl |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 76baa8e..1434eb2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -367,10 +367,11 @@ if ($thread && !defined $initial_reply_to && $prompting) {
 	} while (!defined $_);
 
 	$initial_reply_to = $_;
-	$initial_reply_to =~ s/^\s+<?/</;
-	$initial_reply_to =~ s/>?\s+$/>/;
 }
 
+$initial_reply_to =~ s/^\s*\<?/\</;
+$initial_reply_to =~ s/>?\s*$/>/;
+
 if (!defined $smtp_server) {
 	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
 		if (-x $_) {
-- 
1.5.3.7

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Mike Hommey]

On Sun, Dec 09, 2007 at 06:17:28PM +0100, Mike Hommey wrote:
> 3803bcea tried to fix this, but it only adds the branckes when the given

s/branckes/brackets/

Mike

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> 3803bcea tried to fix this, but it only adds the branckes when the given
> In-Reply-To begins and ends with whitespaces. It also didn't do anything
> to the --in-reply-to argument.
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
> I just got bitten by this...

Interesting.

>
>  git-send-email.perl |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 76baa8e..1434eb2 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -367,10 +367,11 @@ if ($thread && !defined $initial_reply_to && $prompting) {
>  	} while (!defined $_);
>  
>  	$initial_reply_to = $_;
> -	$initial_reply_to =~ s/^\s+<?/</;
> -	$initial_reply_to =~ s/>?\s+$/>/;
>  }

I wonder what the original rationale for these \s+ was.
Will apply, anyway.  Thanks.

>  
> +$initial_reply_to =~ s/^\s*\<?/\</;
> +$initial_reply_to =~ s/>?\s*$/>/;
> +
>  if (!defined $smtp_server) {
>  	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
>  		if (-x $_) {
> -- 
> 1.5.3.7

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Mike Hommey]

On Sun, Dec 09, 2007 at 10:09:44AM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> >  git-send-email.perl |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index 76baa8e..1434eb2 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -367,10 +367,11 @@ if ($thread && !defined $initial_reply_to && $prompting) {
> >  	} while (!defined $_);
> >  
> >  	$initial_reply_to = $_;
> > -	$initial_reply_to =~ s/^\s+<?/</;
> > -	$initial_reply_to =~ s/>?\s+$/>/;
> >  }
> 
> I wonder what the original rationale for these \s+ was.

I'd say copy/paste:
-       $initial_reply_to =~ s/(^\s+|\s+$)//g;
+       $initial_reply_to =~ s/^\s+<?/</;
+       $initial_reply_to =~ s/>?\s+$/>/;

Mike

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Mike Hommey <mh@glandium.org> writes:
>
>> 3803bcea tried to fix this, but it only adds the branckes when the given
>> In-Reply-To begins and ends with whitespaces. It also didn't do anything
>> to the --in-reply-to argument.
>>
>> Signed-off-by: Mike Hommey <mh@glandium.org>
>> ---
>>
>> I just got bitten by this...
>
> Interesting.

Ouch.

>>  git-send-email.perl |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 76baa8e..1434eb2 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -367,10 +367,11 @@ if ($thread && !defined $initial_reply_to && $prompting) {
>>  	} while (!defined $_);
>>  
>>  	$initial_reply_to = $_;
>> -	$initial_reply_to =~ s/^\s+<?/</;
>> -	$initial_reply_to =~ s/>?\s+$/>/;
>>  }
>
> I wonder what the original rationale for these \s+ was.
> Will apply, anyway.  Thanks.

The original line read

	$initial_reply_to =~ s/(^\s+|\s+$)//g;

and was used just for stripping spaces (no stripping necessary when
there is no space, so \s+ was ok).  The change was supposed to work
on the brackets, too.

That Mike got bitten here is proof that the original idea had merit.
Too bad the implementation did not actually work.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Randal L. Schwartz]

>>>>> "David" == David Kastrup <dak@gnu.org> writes:

>>> $initial_reply_to = $_;
>>> -	$initial_reply_to =~ s/^\s+<?/</;
>>> -	$initial_reply_to =~ s/>?\s+$/>/;
>>> }
>> 
>> I wonder what the original rationale for these \s+ was.
>> Will apply, anyway.  Thanks.

David> The original line read

David> 	$initial_reply_to =~ s/(^\s+|\s+$)//g;

David> and was used just for stripping spaces (no stripping necessary when
David> there is no space, so \s+ was ok).  The change was supposed to work
David> on the brackets, too.

David> That Mike got bitten here is proof that the original idea had merit.
David> Too bad the implementation did not actually work.

I think what you were trying to do would work with:

  for ($initial_reply_to) {
      s/^\s*<?/</;
      s/>?\s*$/>/;
  }

Untested, but I get this stuff right most of the time. :)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[David Kastrup]

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> I think what you were trying to do would work with:
>
>   for ($initial_reply_to) {
>       s/^\s*<?/</;
>       s/>?\s*$/>/;
>   }
>
> Untested, but I get this stuff right most of the time. :)

How is that supposed to differ from Mike's patch?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Randal L. Schwartz]

>>>>> "David" == David Kastrup <dak@gnu.org> writes:

David> How is that supposed to differ from Mike's patch?

Now that I've seen it, not a bit.  Sorry, was solving the problem that had
already been solved, in a slightly different way.

Oh, you don't need to \ the < and > though.  Cleans it up a bit
if you get rid of those.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

[Resend PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary

[Mike Hommey]

3803bcea tried to fix this, but it only adds the branckes when the given
In-Reply-To begins and ends with whitespaces. It also didn't do anything
to the --in-reply-to argument.

Signed-off-by: Mike Hommey <mh@glandium.org>
---

Randal L. Schwartz wrote:
> Oh, you don't need to \ the < and > though.  Cleans it up a bit
> if you get rid of those.

Doh', I forgot to remove them. Here is a cleaned out patch.

 git-send-email.perl |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 76baa8e..c0e1dd3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -367,10 +367,11 @@ if ($thread && !defined $initial_reply_to && $prompting) {
 	} while (!defined $_);
 
 	$initial_reply_to = $_;
-	$initial_reply_to =~ s/^\s+<?/</;
-	$initial_reply_to =~ s/>?\s+$/>/;
 }
 
+$initial_reply_to =~ s/^\s*<?/</;
+$initial_reply_to =~ s/>?\s*$/>/;
+
 if (!defined $smtp_server) {
 	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
 		if (-x $_) {
-- 
1.5.3.7

Re: [PATCH 1/4] Cleanup variables in http.[ch]

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> Quite some variables defined as extern in http.h are only used in http.c,
> and some others, only defined in http.c, were not static.

Good riddance of many globals.