[PATCH 1/5] Remove the default_headers variable from http-push.c

[PATCH 1/5] Remove the default_headers variable from http-push.c

[Mike Hommey]

It appears that despite being initialized, it was never used.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 http-push.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/http-push.c b/http-push.c
index a69d0e3..2ef764c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -75,7 +75,6 @@ static int aborted;
 static signed char remote_dir_exists[256];
 
 static struct curl_slist *no_pragma_header;
-static struct curl_slist *default_headers;
 
 static int push_verbosely;
 static int push_all = MATCH_REFS_NONE;
@@ -2286,11 +2285,6 @@ int main(int argc, char **argv)
 	http_init();
 
 	no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
-	default_headers = curl_slist_append(default_headers, "Range:");
-	default_headers = curl_slist_append(default_headers, "Destination:");
-	default_headers = curl_slist_append(default_headers, "If:");
-	default_headers = curl_slist_append(default_headers,
-					    "Pragma: no-cache");
 
 	/* Verify DAV compliance/lock support */
 	if (!locking_available()) {
@@ -2470,7 +2464,6 @@ int main(int argc, char **argv)
 	free(remote);
 
 	curl_slist_free_all(no_pragma_header);
-	curl_slist_free_all(default_headers);
 
 	http_cleanup();
 
-- 
1.5.3.7.1159.gdd4a4

[PATCH 2/5] Remove a CURLOPT_HTTPHEADER (un)setting

[Mike Hommey]

Setting CURLOPT_HTTPHEADER doesn't add HTTP headers, but replaces whatever
set of headers was configured before, so setting to NULL doesn't have any
magic meaning, and is pretty much useless when setting to another list
right after.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 http.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/http.c b/http.c
index 2d0b46d..784b93e 100644
--- a/http.c
+++ b/http.c
@@ -364,7 +364,6 @@ struct active_request_slot *get_active_slot(void)
 	slot->finished = NULL;
 	slot->callback_data = NULL;
 	slot->callback_func = NULL;
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);
 	curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);
 	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);
-- 
1.5.3.7.1159.gdd4a4

[PATCH 3/5] Avoid redundant declaration of missing_target()

[Mike Hommey]

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

I though it's also small enough to grant inlining.

 http-walker.c |   13 -------------
 http.h        |   13 +++++++++++++
 transport.c   |   13 -------------
 3 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/http-walker.c b/http-walker.c
index d9a5f1e..8dbf9cc 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -90,19 +90,6 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb,
 	return size;
 }
 
-static int missing__target(int code, int result)
-{
-	return	/* file:// URL -- do we ever use one??? */
-		(result == CURLE_FILE_COULDNT_READ_FILE) ||
-		/* http:// and https:// URL */
-		(code == 404 && result == CURLE_HTTP_RETURNED_ERROR) ||
-		/* ftp:// URL */
-		(code == 550 && result == CURLE_FTP_COULDNT_RETR_FILE)
-		;
-}
-
-#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
-
 static void fetch_alternates(struct walker *walker, const char *base);
 
 static void process_object_response(void *callback_data);
diff --git a/http.h b/http.h
index a0fb4cf..87d638b 100644
--- a/http.h
+++ b/http.h
@@ -83,4 +83,17 @@ extern int active_requests;
 
 extern char curl_errorstr[CURL_ERROR_SIZE];
 
+static inline int missing__target(int code, int result)
+{
+	return	/* file:// URL -- do we ever use one??? */
+		(result == CURLE_FILE_COULDNT_READ_FILE) ||
+		/* http:// and https:// URL */
+		(code == 404 && result == CURLE_HTTP_RETURNED_ERROR) ||
+		/* ftp:// URL */
+		(code == 550 && result == CURLE_FTP_COULDNT_RETR_FILE)
+		;
+}
+
+#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
+
 #endif /* HTTP_H */
diff --git a/transport.c b/transport.c
index 22234e8..4e151a9 100644
--- a/transport.c
+++ b/transport.c
@@ -426,19 +426,6 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons
 	return !!err;
 }
 
-static int missing__target(int code, int result)
-{
-	return	/* file:// URL -- do we ever use one??? */
-		(result == CURLE_FILE_COULDNT_READ_FILE) ||
-		/* http:// and https:// URL */
-		(code == 404 && result == CURLE_HTTP_RETURNED_ERROR) ||
-		/* ftp:// URL */
-		(code == 550 && result == CURLE_FTP_COULDNT_RETR_FILE)
-		;
-}
-
-#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
-
 static struct ref *get_refs_via_curl(struct transport *transport)
 {
 	struct strbuf buffer = STRBUF_INIT;
-- 
1.5.3.7.1159.gdd4a4

[PATCH 4/5] Correctly initialize buffer in start_put() in http-push.c

[Mike Hommey]

Brown paper bag fix to avoid random misbehaviour.

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

 This is a brown paper bag fix for my strbuf patch for the http code.

 http-push.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/http-push.c b/http-push.c
index 2ef764c..ad8167e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -495,6 +495,7 @@ static void start_put(struct transfer_request *request)
 	deflateInit(&stream, zlib_compression_level);
 	size = deflateBound(&stream, len + hdrlen);
 	strbuf_init(&request->buffer.buf, size);
+	request->buffer.posn = 0;
 
 	/* Compress it */
 	stream.next_out = (unsigned char *)request->buffer.buf.buf;
-- 
1.5.3.7.1159.gdd4a4

[PATCH 5/5] Fix various memory leaks in http-push.c and http-walker.c

[Mike Hommey]

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

 This one, too, sits on top of my strbuf patch for the http code. Note that
 I only went for the obvious ones I saw in the code I touched. There is more
 in these files.

 http-push.c   |   32 +++++++++++++++++++++-----------
 http-walker.c |   40 +++++++++++++++++++++++++---------------
 2 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/http-push.c b/http-push.c
index ad8167e..610ed9c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -924,6 +924,7 @@ static int fetch_index(unsigned char *sha1)
 				     hex);
 		}
 	} else {
+		free(url);
 		return error("Unable to start request");
 	}
 
@@ -1114,6 +1115,7 @@ int fetch_ref(char *ref, unsigned char *sha1)
 	char *base = remote->url;
 	struct active_request_slot *slot;
 	struct slot_results results;
+	int ret;
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -1124,17 +1126,23 @@ int fetch_ref(char *ref, unsigned char *sha1)
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
-		if (results.curl_result != CURLE_OK)
-			return error("Couldn't get %s for %s\n%s",
-				     url, ref, curl_errorstr);
+		if (results.curl_result == CURLE_OK) {
+			strbuf_rtrim(&buffer);
+			if (buffer.len == 40)
+				ret = get_sha1_hex(buffer.buf, sha1);
+			else
+				ret = 1;
+		} else {
+			ret = error("Couldn't get %s for %s\n%s",
+				    url, ref, curl_errorstr);
+		}
 	} else {
-		return error("Unable to start request");
+		ret = error("Unable to start request");
 	}
 
-	strbuf_rtrim(&buffer);
-	if (buffer.len != 40)
-		return 1;
-	return get_sha1_hex(buffer.buf, sha1);
+	strbuf_release(&buffer);
+	free(url);
+	return ret;
 }
 
 static void one_remote_object(const char *hex)
@@ -2033,6 +2041,7 @@ static int remote_exists(const char *path)
 	char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
 	struct active_request_slot *slot;
 	struct slot_results results;
+	int ret = -1;
 
 	sprintf(url, "%s%s", remote->url, path);
 
@@ -2044,16 +2053,17 @@ static int remote_exists(const char *path)
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.http_code == 404)
-			return 0;
+			ret = 0;
 		else if (results.curl_result == CURLE_OK)
-			return 1;
+			ret = 1;
 		else
 			fprintf(stderr, "HEAD HTTP error %ld\n", results.http_code);
 	} else {
 		fprintf(stderr, "Unable to start HEAD request\n");
 	}
 
-	return -1;
+	free(url);
+	return ret;
 }
 
 static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
diff --git a/http-walker.c b/http-walker.c
index 8dbf9cc..4e878b3 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -644,6 +644,7 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 	struct strbuf buffer = STRBUF_INIT;
 	char *data;
 	int i = 0;
+	int ret = 0;
 
 	struct active_request_slot *slot;
 	struct slot_results results;
@@ -666,19 +667,19 @@ 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;
-				return 0;
+				goto cleanup;
 			} else {
 				repo->got_indices = 0;
-				return error("%s", curl_errorstr);
+				ret = error("%s", curl_errorstr);
+				goto cleanup;
 			}
 		}
 	} else {
 		repo->got_indices = 0;
-		strbuf_release(&buffer);
-		return error("Unable to start request");
+		ret = error("Unable to start request");
+		goto cleanup;
 	}
 
 	data = buffer.buf;
@@ -701,9 +702,11 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
 		i++;
 	}
 
-	strbuf_release(&buffer);
 	repo->got_indices = 1;
-	return 0;
+cleanup:
+	strbuf_release(&buffer);
+	free(url);
+	return ret;
 }
 
 static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned char *sha1)
@@ -939,6 +942,7 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 	const char *base = data->alt->base;
 	struct active_request_slot *slot;
 	struct slot_results results;
+	int ret;
 
 	url = quote_ref_url(base, ref);
 	slot = get_active_slot();
@@ -949,17 +953,23 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
-		if (results.curl_result != CURLE_OK)
-			return error("Couldn't get %s for %s\n%s",
-				     url, ref, curl_errorstr);
+		if (results.curl_result == CURLE_OK) {
+			strbuf_rtrim(&buffer);
+			if (buffer.len == 40)
+				ret = get_sha1_hex(buffer.buf, sha1);
+			else
+				ret = 1;
+		} else {
+			ret = error("Couldn't get %s for %s\n%s",
+				    url, ref, curl_errorstr);
+		}
 	} else {
-		return error("Unable to start request");
+		ret = error("Unable to start request");
 	}
 
-	strbuf_rtrim(&buffer);
-	if (buffer.len != 40)
-		return 1;
-	return get_sha1_hex(buffer.buf, sha1);
+	strbuf_release(&buffer);
+	free(url);
+	return ret;
 }
 
 static void cleanup(struct walker *walker)
-- 
1.5.3.7.1159.gdd4a4

[PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c

[Mike Hommey]

Make the necessary changes to be ok with their difference, and rename the
function http_fetch_ref.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 http-push.c   |   88 ++------------------------------------------------------
 http-walker.c |   80 +---------------------------------------------------
 http.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h        |    2 +
 4 files changed, 89 insertions(+), 163 deletions(-)

diff --git a/http-push.c b/http-push.c
index 610ed9c..a4a9d1c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1063,88 +1063,6 @@ static int fetch_indices(void)
 	return 0;
 }
 
-static inline int needs_quote(int ch)
-{
-	if (((ch >= 'A') && (ch <= 'Z'))
-			|| ((ch >= 'a') && (ch <= 'z'))
-			|| ((ch >= '0') && (ch <= '9'))
-			|| (ch == '/')
-			|| (ch == '-')
-			|| (ch == '.'))
-		return 0;
-	return 1;
-}
-
-static inline int hex(int v)
-{
-	if (v < 10) return '0' + v;
-	else return 'A' + v - 10;
-}
-
-static char *quote_ref_url(const char *base, const char *ref)
-{
-	const char *cp;
-	char *dp, *qref;
-	int len, baselen, ch;
-
-	baselen = strlen(base);
-	len = baselen + 1;
-	for (cp = ref; (ch = *cp) != 0; cp++, len++)
-		if (needs_quote(ch))
-			len += 2; /* extra two hex plus replacement % */
-	qref = xmalloc(len);
-	memcpy(qref, base, baselen);
-	for (cp = ref, dp = qref + baselen; (ch = *cp) != 0; cp++) {
-		if (needs_quote(ch)) {
-			*dp++ = '%';
-			*dp++ = hex((ch >> 4) & 0xF);
-			*dp++ = hex(ch & 0xF);
-		}
-		else
-			*dp++ = ch;
-	}
-	*dp = 0;
-
-	return qref;
-}
-
-int fetch_ref(char *ref, unsigned char *sha1)
-{
-	char *url;
-	struct strbuf buffer = STRBUF_INIT;
-	char *base = remote->url;
-	struct active_request_slot *slot;
-	struct slot_results results;
-	int ret;
-
-	url = quote_ref_url(base, ref);
-	slot = get_active_slot();
-	slot->results = &results;
-	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
-	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-	if (start_active_slot(slot)) {
-		run_active_slot(slot);
-		if (results.curl_result == CURLE_OK) {
-			strbuf_rtrim(&buffer);
-			if (buffer.len == 40)
-				ret = get_sha1_hex(buffer.buf, sha1);
-			else
-				ret = 1;
-		} else {
-			ret = error("Couldn't get %s for %s\n%s",
-				    url, ref, curl_errorstr);
-		}
-	} else {
-		ret = error("Unable to start request");
-	}
-
-	strbuf_release(&buffer);
-	free(url);
-	return ret;
-}
-
 static void one_remote_object(const char *hex)
 {
 	unsigned char sha1[20];
@@ -1827,7 +1745,8 @@ static void one_remote_ref(char *refname)
 	struct object *obj;
 	int len = strlen(refname) + 1;
 
-	if (fetch_ref(refname, remote_sha1) != 0) {
+	if (http_fetch_ref(remote->url, refname + 5 /* "refs/" */,
+			   remote_sha1) != 0) {
 		fprintf(stderr,
 			"Unable to fetch ref %s from %s\n",
 			refname, remote->url);
@@ -1959,7 +1878,8 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
 	int len;
 	char *ref_info;
 
-	if (fetch_ref(ls->dentry_name, remote_sha1) != 0) {
+	if (http_fetch_ref(remote->url, ls->dentry_name + 5 /* "refs/" */,
+			   remote_sha1) != 0) {
 		fprintf(stderr,
 			"Unable to fetch ref %s from %s\n",
 			ls->dentry_name, remote->url);
diff --git a/http-walker.c b/http-walker.c
index 4e878b3..2c37868 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -888,88 +888,10 @@ static int fetch(struct walker *walker, unsigned char *sha1)
 		     data->alt->base);
 }
 
-static inline int needs_quote(int ch)
-{
-	if (((ch >= 'A') && (ch <= 'Z'))
-			|| ((ch >= 'a') && (ch <= 'z'))
-			|| ((ch >= '0') && (ch <= '9'))
-			|| (ch == '/')
-			|| (ch == '-')
-			|| (ch == '.'))
-		return 0;
-	return 1;
-}
-
-static inline int hex(int v)
-{
-	if (v < 10) return '0' + v;
-	else return 'A' + v - 10;
-}
-
-static char *quote_ref_url(const char *base, const char *ref)
-{
-	const char *cp;
-	char *dp, *qref;
-	int len, baselen, ch;
-
-	baselen = strlen(base);
-	len = baselen + 7; /* "/refs/" + NUL */
-	for (cp = ref; (ch = *cp) != 0; cp++, len++)
-		if (needs_quote(ch))
-			len += 2; /* extra two hex plus replacement % */
-	qref = xmalloc(len);
-	memcpy(qref, base, baselen);
-	memcpy(qref + baselen, "/refs/", 6);
-	for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
-		if (needs_quote(ch)) {
-			*dp++ = '%';
-			*dp++ = hex((ch >> 4) & 0xF);
-			*dp++ = hex(ch & 0xF);
-		}
-		else
-			*dp++ = ch;
-	}
-	*dp = 0;
-
-	return qref;
-}
-
 static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 {
-	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;
-	int ret;
-
-	url = quote_ref_url(base, ref);
-	slot = get_active_slot();
-	slot->results = &results;
-	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
-	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-	if (start_active_slot(slot)) {
-		run_active_slot(slot);
-		if (results.curl_result == CURLE_OK) {
-			strbuf_rtrim(&buffer);
-			if (buffer.len == 40)
-				ret = get_sha1_hex(buffer.buf, sha1);
-			else
-				ret = 1;
-		} else {
-			ret = error("Couldn't get %s for %s\n%s",
-				    url, ref, curl_errorstr);
-		}
-	} else {
-		ret = error("Unable to start request");
-	}
-
-	strbuf_release(&buffer);
-	free(url);
-	return ret;
+	return http_fetch_ref(data->alt->base, ref, sha1);
 }
 
 static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 784b93e..c6de964 100644
--- a/http.c
+++ b/http.c
@@ -552,3 +552,85 @@ void finish_all_active_slots(void)
 			slot = slot->next;
 		}
 }
+
+static inline int needs_quote(int ch)
+{
+	if (((ch >= 'A') && (ch <= 'Z'))
+			|| ((ch >= 'a') && (ch <= 'z'))
+			|| ((ch >= '0') && (ch <= '9'))
+			|| (ch == '/')
+			|| (ch == '-')
+			|| (ch == '.'))
+		return 0;
+	return 1;
+}
+
+static inline int hex(int v)
+{
+	if (v < 10) return '0' + v;
+	else return 'A' + v - 10;
+}
+
+static char *quote_ref_url(const char *base, const char *ref)
+{
+	const char *cp;
+	char *dp, *qref;
+	int len, baselen, ch;
+
+	baselen = strlen(base);
+	len = baselen + 7; /* "/refs/" + NUL */
+	for (cp = ref; (ch = *cp) != 0; cp++, len++)
+		if (needs_quote(ch))
+			len += 2; /* extra two hex plus replacement % */
+	qref = xmalloc(len);
+	memcpy(qref, base, baselen);
+	memcpy(qref + baselen, "/refs/", 6);
+	for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
+		if (needs_quote(ch)) {
+			*dp++ = '%';
+			*dp++ = hex((ch >> 4) & 0xF);
+			*dp++ = hex(ch & 0xF);
+		}
+		else
+			*dp++ = ch;
+	}
+	*dp = 0;
+
+	return qref;
+}
+
+int http_fetch_ref(const char *base, const char *ref, unsigned char *sha1)
+{
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
+	struct active_request_slot *slot;
+	struct slot_results results;
+	int ret;
+
+	url = quote_ref_url(base, ref);
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
+	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	if (start_active_slot(slot)) {
+		run_active_slot(slot);
+		if (results.curl_result == CURLE_OK) {
+			strbuf_rtrim(&buffer);
+			if (buffer.len == 40)
+				ret = get_sha1_hex(buffer.buf, sha1);
+			else
+				ret = 1;
+		} else {
+			ret = error("Couldn't get %s for %s\n%s",
+				    url, ref, curl_errorstr);
+		}
+	} else {
+		ret = error("Unable to start request");
+	}
+
+	strbuf_release(&buffer);
+	free(url);
+	return ret;
+}
diff --git a/http.h b/http.h
index 87d638b..b709222 100644
--- a/http.h
+++ b/http.h
@@ -96,4 +96,6 @@ static inline int missing__target(int code, int result)
 
 #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
 
+extern int http_fetch_ref(const char *base, const char *ref, unsigned char *sha1);
+
 #endif /* HTTP_H */
-- 
1.5.3.7.1160.g1e7a-dirty

Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> Make the necessary changes to be ok with their difference, and rename the
> function http_fetch_ref.

Sorry, but I cannot parse "ok with their difference" part...

Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c

[Mike Hommey]

On Mon, Dec 10, 2007 at 09:09:05PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > Make the necessary changes to be ok with their difference, and rename the
> > function http_fetch_ref.
> 
> Sorry, but I cannot parse "ok with their difference" part...

What about "to fit with their difference" ?

Mike

Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c

[Andreas Ericsson]

Mike Hommey wrote:
> On Mon, Dec 10, 2007 at 09:09:05PM -0800, Junio C Hamano wrote:
>> Mike Hommey <mh@glandium.org> writes:
>>
>>> Make the necessary changes to be ok with their difference, and rename the
>>> function http_fetch_ref.
>> Sorry, but I cannot parse "ok with their difference" part...
> 
> What about "to fit with their difference" ?
> 

Define "their" and "difference". Implicitly referring to another commit
message won't help cursory glances much (and I have no idea what you're
talking about since I didn't read the rest of your series very carefully).

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c

[Mike Hommey]

On Tue, Dec 11, 2007 at 08:22:08AM +0100, Andreas Ericsson wrote:
> Mike Hommey wrote:
>> On Mon, Dec 10, 2007 at 09:09:05PM -0800, Junio C Hamano wrote:
>>> Mike Hommey <mh@glandium.org> writes:
>>>
>>>> Make the necessary changes to be ok with their difference, and rename the
>>>> function http_fetch_ref.
>>> Sorry, but I cannot parse "ok with their difference" part...
>>
>> What about "to fit with their difference" ?
>>
>
> Define "their" and "difference". Implicitly referring to another commit
> message won't help cursory glances much (and I have no idea what you're
> talking about since I didn't read the rest of your series very carefully).

The rest of the series doesn't have anything to do with this one, except
that fetch_ref had been modified to use strbuf in both.

Their difference is that one would take a ref with "refs/" at the
beginning and the other wouldn't. I'll try to write a better commit
message later today.

Mike

[New PATCH] Move fetch_ref from http-push.c and http-walker.c to http.c

[Mike Hommey]

Call the moved function http_fetch_ref instead.
Also move needs_quote, hex and quote_ref_url altogether.

Both implementations had a slight difference, so it is not only code moving:

- fetch_ref in http-push.c expects the ref to include "refs/" at the
  beginning of its name, and gets the base url from a global variable.
- fetch_ref in http-walker.c expects the ref to *not* include "refs/" at
  the beginning (quote_ref_url would add it back), and gets the base url
  from a struct walker * argument.

So we actually leave a fetch_ref wrapper function in http-walker.c that
calls the new http_fetch_ref with the proper arguments. In http-push.c,
we just change the only 2 callers to use the new function, with "refs/"
removed from the ref argument.

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

  Only change is the commit message.

 http-push.c   |   88 ++------------------------------------------------------
 http-walker.c |   80 +---------------------------------------------------
 http.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h        |    2 +
 4 files changed, 89 insertions(+), 163 deletions(-)

diff --git a/http-push.c b/http-push.c
index 610ed9c..a4a9d1c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1063,88 +1063,6 @@ static int fetch_indices(void)
 	return 0;
 }
 
-static inline int needs_quote(int ch)
-{
-	if (((ch >= 'A') && (ch <= 'Z'))
-			|| ((ch >= 'a') && (ch <= 'z'))
-			|| ((ch >= '0') && (ch <= '9'))
-			|| (ch == '/')
-			|| (ch == '-')
-			|| (ch == '.'))
-		return 0;
-	return 1;
-}
-
-static inline int hex(int v)
-{
-	if (v < 10) return '0' + v;
-	else return 'A' + v - 10;
-}
-
-static char *quote_ref_url(const char *base, const char *ref)
-{
-	const char *cp;
-	char *dp, *qref;
-	int len, baselen, ch;
-
-	baselen = strlen(base);
-	len = baselen + 1;
-	for (cp = ref; (ch = *cp) != 0; cp++, len++)
-		if (needs_quote(ch))
-			len += 2; /* extra two hex plus replacement % */
-	qref = xmalloc(len);
-	memcpy(qref, base, baselen);
-	for (cp = ref, dp = qref + baselen; (ch = *cp) != 0; cp++) {
-		if (needs_quote(ch)) {
-			*dp++ = '%';
-			*dp++ = hex((ch >> 4) & 0xF);
-			*dp++ = hex(ch & 0xF);
-		}
-		else
-			*dp++ = ch;
-	}
-	*dp = 0;
-
-	return qref;
-}
-
-int fetch_ref(char *ref, unsigned char *sha1)
-{
-	char *url;
-	struct strbuf buffer = STRBUF_INIT;
-	char *base = remote->url;
-	struct active_request_slot *slot;
-	struct slot_results results;
-	int ret;
-
-	url = quote_ref_url(base, ref);
-	slot = get_active_slot();
-	slot->results = &results;
-	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
-	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-	if (start_active_slot(slot)) {
-		run_active_slot(slot);
-		if (results.curl_result == CURLE_OK) {
-			strbuf_rtrim(&buffer);
-			if (buffer.len == 40)
-				ret = get_sha1_hex(buffer.buf, sha1);
-			else
-				ret = 1;
-		} else {
-			ret = error("Couldn't get %s for %s\n%s",
-				    url, ref, curl_errorstr);
-		}
-	} else {
-		ret = error("Unable to start request");
-	}
-
-	strbuf_release(&buffer);
-	free(url);
-	return ret;
-}
-
 static void one_remote_object(const char *hex)
 {
 	unsigned char sha1[20];
@@ -1827,7 +1745,8 @@ static void one_remote_ref(char *refname)
 	struct object *obj;
 	int len = strlen(refname) + 1;
 
-	if (fetch_ref(refname, remote_sha1) != 0) {
+	if (http_fetch_ref(remote->url, refname + 5 /* "refs/" */,
+			   remote_sha1) != 0) {
 		fprintf(stderr,
 			"Unable to fetch ref %s from %s\n",
 			refname, remote->url);
@@ -1959,7 +1878,8 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
 	int len;
 	char *ref_info;
 
-	if (fetch_ref(ls->dentry_name, remote_sha1) != 0) {
+	if (http_fetch_ref(remote->url, ls->dentry_name + 5 /* "refs/" */,
+			   remote_sha1) != 0) {
 		fprintf(stderr,
 			"Unable to fetch ref %s from %s\n",
 			ls->dentry_name, remote->url);
diff --git a/http-walker.c b/http-walker.c
index 4e878b3..2c37868 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -888,88 +888,10 @@ static int fetch(struct walker *walker, unsigned char *sha1)
 		     data->alt->base);
 }
 
-static inline int needs_quote(int ch)
-{
-	if (((ch >= 'A') && (ch <= 'Z'))
-			|| ((ch >= 'a') && (ch <= 'z'))
-			|| ((ch >= '0') && (ch <= '9'))
-			|| (ch == '/')
-			|| (ch == '-')
-			|| (ch == '.'))
-		return 0;
-	return 1;
-}
-
-static inline int hex(int v)
-{
-	if (v < 10) return '0' + v;
-	else return 'A' + v - 10;
-}
-
-static char *quote_ref_url(const char *base, const char *ref)
-{
-	const char *cp;
-	char *dp, *qref;
-	int len, baselen, ch;
-
-	baselen = strlen(base);
-	len = baselen + 7; /* "/refs/" + NUL */
-	for (cp = ref; (ch = *cp) != 0; cp++, len++)
-		if (needs_quote(ch))
-			len += 2; /* extra two hex plus replacement % */
-	qref = xmalloc(len);
-	memcpy(qref, base, baselen);
-	memcpy(qref + baselen, "/refs/", 6);
-	for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
-		if (needs_quote(ch)) {
-			*dp++ = '%';
-			*dp++ = hex((ch >> 4) & 0xF);
-			*dp++ = hex(ch & 0xF);
-		}
-		else
-			*dp++ = ch;
-	}
-	*dp = 0;
-
-	return qref;
-}
-
 static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
 {
-	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;
-	int ret;
-
-	url = quote_ref_url(base, ref);
-	slot = get_active_slot();
-	slot->results = &results;
-	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
-	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
-	if (start_active_slot(slot)) {
-		run_active_slot(slot);
-		if (results.curl_result == CURLE_OK) {
-			strbuf_rtrim(&buffer);
-			if (buffer.len == 40)
-				ret = get_sha1_hex(buffer.buf, sha1);
-			else
-				ret = 1;
-		} else {
-			ret = error("Couldn't get %s for %s\n%s",
-				    url, ref, curl_errorstr);
-		}
-	} else {
-		ret = error("Unable to start request");
-	}
-
-	strbuf_release(&buffer);
-	free(url);
-	return ret;
+	return http_fetch_ref(data->alt->base, ref, sha1);
 }
 
 static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 784b93e..c6de964 100644
--- a/http.c
+++ b/http.c
@@ -552,3 +552,85 @@ void finish_all_active_slots(void)
 			slot = slot->next;
 		}
 }
+
+static inline int needs_quote(int ch)
+{
+	if (((ch >= 'A') && (ch <= 'Z'))
+			|| ((ch >= 'a') && (ch <= 'z'))
+			|| ((ch >= '0') && (ch <= '9'))
+			|| (ch == '/')
+			|| (ch == '-')
+			|| (ch == '.'))
+		return 0;
+	return 1;
+}
+
+static inline int hex(int v)
+{
+	if (v < 10) return '0' + v;
+	else return 'A' + v - 10;
+}
+
+static char *quote_ref_url(const char *base, const char *ref)
+{
+	const char *cp;
+	char *dp, *qref;
+	int len, baselen, ch;
+
+	baselen = strlen(base);
+	len = baselen + 7; /* "/refs/" + NUL */
+	for (cp = ref; (ch = *cp) != 0; cp++, len++)
+		if (needs_quote(ch))
+			len += 2; /* extra two hex plus replacement % */
+	qref = xmalloc(len);
+	memcpy(qref, base, baselen);
+	memcpy(qref + baselen, "/refs/", 6);
+	for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
+		if (needs_quote(ch)) {
+			*dp++ = '%';
+			*dp++ = hex((ch >> 4) & 0xF);
+			*dp++ = hex(ch & 0xF);
+		}
+		else
+			*dp++ = ch;
+	}
+	*dp = 0;
+
+	return qref;
+}
+
+int http_fetch_ref(const char *base, const char *ref, unsigned char *sha1)
+{
+	char *url;
+	struct strbuf buffer = STRBUF_INIT;
+	struct active_request_slot *slot;
+	struct slot_results results;
+	int ret;
+
+	url = quote_ref_url(base, ref);
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
+	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	if (start_active_slot(slot)) {
+		run_active_slot(slot);
+		if (results.curl_result == CURLE_OK) {
+			strbuf_rtrim(&buffer);
+			if (buffer.len == 40)
+				ret = get_sha1_hex(buffer.buf, sha1);
+			else
+				ret = 1;
+		} else {
+			ret = error("Couldn't get %s for %s\n%s",
+				    url, ref, curl_errorstr);
+		}
+	} else {
+		ret = error("Unable to start request");
+	}
+
+	strbuf_release(&buffer);
+	free(url);
+	return ret;
+}
diff --git a/http.h b/http.h
index 87d638b..b709222 100644
--- a/http.h
+++ b/http.h
@@ -96,4 +96,6 @@ static inline int missing__target(int code, int result)
 
 #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
 
+extern int http_fetch_ref(const char *base, const char *ref, unsigned char *sha1);
+
 #endif /* HTTP_H */
-- 
1.5.3.7.1161.g4a58