Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-send-email.perl: Really add angle brackets to   In-Reply-To if necessary
From: Randal L. Schwartz @ 2007-12-09 19:51 UTC (permalink / raw)
  To: David Kastrup; +Cc: Junio C Hamano, Mike Hommey, git
In-Reply-To: <85lk83r6qq.fsf@lola.goethe.zz>

>>>>> "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!

^ permalink raw reply

* Re: [PATCH] git-send-email.perl: Really add angle brackets to  In-Reply-To if necessary
From: David Kastrup @ 2007-12-09 19:46 UTC (permalink / raw)
  To: Randal L. Schwartz; +Cc: Junio C Hamano, Mike Hommey, git
In-Reply-To: <86aboju2bp.fsf@blue.stonehenge.com>

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

^ permalink raw reply

* Re: git-svn branch naming question
From: Miklos Vajna @ 2007-12-09 19:36 UTC (permalink / raw)
  To: Eric Wong; +Cc: Peter Baumann, git
In-Reply-To: <20071209032608.GB31033@soma>

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On Sat, Dec 08, 2007 at 07:26:08PM -0800, Eric Wong <normalperson@yhbt.net> wrote:
> Perhaps git-clone could gain the ability to clone refs/remotes/ as-is
> without an extra step?

well, what i did for now is to use 2 repos. i use git-svn to create a
regular git-svn repo from svn, then an other one to create a regular git
repo from the git-svn one. this has one benefit: git-svn requires a
working tree, but i definitely want to publish only a bare repo. so here
is what i have in the bare repo's config:

[remote "origin"]
        url = /path/to/git-svn/repo
        fetch = +refs/remotes/origin/tags/*:refs/tags/*
        fetch = +refs/remotes/origin/trunk:refs/heads/master
        fetch = +refs/remotes/origin/*:refs/heads/*

then a simple git fetch will do what i need. i'm not entirely sure this
is the right think to do, but works for me :)

thanks,
- VMiklos

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [Resend PATCH 2/4] Use strbuf in http code
From: Mike Hommey @ 2007-12-09 19:30 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <7vy7c3ogu2.fsf@gitster.siamese.dyndns.org>

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

^ permalink raw reply related

* Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.
From: Brian Gernhardt @ 2007-12-09 19:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jim Meyering, git list
In-Reply-To: <7vsl2bpwe1.fsf@gitster.siamese.dyndns.org>


On Dec 9, 2007, at 1:15 PM, Junio C Hamano wrote:

> The fix may be correct but the comment above the part the patch  
> fixes is
> very misleading and I wasted a few minutes checking to see what it is
> doing is correct.

The fix is correct.  I did not anticipate empty strings when I wrote  
the loop in question.  Although I wonder if that should be expanded to  
use isspace instead of checking for just ' '.  I'm less familiar with  
the config parsing than I was.

> The "quote" variable does not really control quoting; the quoting with
> backslash is always done for the value, and this check is about  
> adding a
> dq pair around it, so that the parser does not lose leading or  
> trailing
> SP or string after start-of-comment marker characters.

It's for adding quotes, which made the comment make sense to me.  But  
your expanded description is correct and matches the commit message I  
wrote.  Thanks for clarifying it in the code.

~~ Brian

^ permalink raw reply

* Re: How-to combine several separate git repos?
From: Wink Saville @ 2007-12-09 19:12 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <20071209104336.GA3163@steel.home>

Alex Riesen wrote:
> Wink Saville, Sun, Dec 09, 2007 07:34:01 +0100:
>   
>> I've got several git repositories of different projects and was thinking
>> I should combine into one repository, but my googling around didn't turn up
>> any simple way of doing it.
>>
>> Any advice?
>>     
>
> Should they both be visible in one working tree as directories?
>   
Yes, I currently have:

  ~/prgs/android/StateMachine
  ~/prgs/android/test2
  ~/prgs/android/test-annotation

I'd with 3 git repo's in StateMachine, test2 & test-annotation.
Ideally I was thinking having using submodules as they
separate but related. But not sure submodules are ready.
> Should these be independent branches?
>
> For instance, you can fetch one into another:
>
>     $ cd project1
>     $ git config remote.project2.url /path/to/project2
>     $ git config remote.project2.fetch 'refs/heads/*:refs/project2/*'
>     $ git fetch project
>
> That will give you two (or more) branches, containing history of the
> project1 and project2. They are still completely independent, just use
> the same object store.
>   
So the above isn't what I want, but for grins I tried it, and got to
this point:

wink@ic2d1:$ cd StateMachine
wink@ic2d1:$ cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
wink@ic2d1:$ git config remote.test2.url ../test2
wink@ic2d1:$ git config remote.test2.fetch 'refs/heads/*:refs/test2/*'
wink@ic2d1:$ cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "test2"]
        url = ../test2
        fetch = refs/heads/*:refs/test2/*
wink@ic2d1:$ git fetch test2
warning: no common commits
remote: Counting objects: 24, done.  
remote: Compressing objects: 100% remote: (16/16), done.  
remote: Total 24 (delta 3), reused 0 (delta 0)
Unpacking objects: 100% (24/24), done.  
 From ../test2
 * [new branch]      master -> refs/test2/master
wink@ic2d1:$ git checkout -b test2 test2/master
Branch test2 set up to track remote branch refs/test2/master.
Switched to a new branch "test2"
wink@ic2d1:$ git status
# On branch test2
nothing to commit (working directory clean)
wink@ic2d1:$ git status
# On branch test2
nothing to commit (working directory clean)
wink@ic2d1:$ git branch -a
  master
* test2
wink@ic2d1:$ git checkout master
Switched to branch "master"
wink@ic2d1:$ git checkout test2
Switched to branch "test2"

So that worked, but as I mentioned not quite what I want
as I want to "see" them all simultaneously.

> You can merge them, for example:
>
>     $ cd project1
>     $ git merge project2/master
>   
Starting over (restoring the original from a tar backup)
this didn't work I get:

wink@ic2d1:$ cd StateMachine
wink@ic2d1:$ git merge ../test2/master
../test2/master - not something we can merge

> Assuming that there is no filename collisions you'll get a repo with
> two merged histories (and two starting points). In case you get
> conflicts you can either resolve them by editing or just move the
> problematic project in subdirectory:
>
>     $ git merge -s ours --no-commit project2/master
>     Automatic merge went well; stopped before committing as requested
>
> here will be no conflicts. Merge strategy "ours" (-s ours) does not
> take anything from the branch to be merged. The coolest strategy ever.
> "--no-commit" stops the operation just before committing.
>
>     $ git read-tree --prefix=project2/ project2/master
>     $ git checkout-index -a
>     $ git commit
>
> That's it. The histories are merged and the files of project2 are
> placed in the directory "project2". It is a wee bit harder to browse
> the history of the files: you have to give both new and "old" name of
> the project2's files, as if you renamed them (that's what read-tree
> with --prefix did).
>
>   
So the first suggestion works, but I don't want them as
separate branches as I want to work on the simultaneously
and they'll share common code.

Another option I was thinking would work for me would be to use
submodules. But I'm not sure submodules are ready for
neophytes and maybe it doesn't do what I want?

Thanks,

Wink Saville

^ permalink raw reply

* Re: [PATCH] git-send-email.perl: Really add angle brackets to  In-Reply-To if necessary
From: Randal L. Schwartz @ 2007-12-09 18:53 UTC (permalink / raw)
  To: David Kastrup; +Cc: Junio C Hamano, Mike Hommey, git
In-Reply-To: <851w9vsp8o.fsf@lola.goethe.zz>

>>>>> "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!

^ permalink raw reply

* Re: [PATCH 2/4] Use strbuf in http code
From: Junio C Hamano @ 2007-12-09 18:36 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, Nick Hengeveld
In-Reply-To: <20071209182408.GA9427@glandium.org>

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.

^ permalink raw reply

* Re: [PATCH] Remove .git/branches from the .git template.
From: Junio C Hamano @ 2007-12-09 18:30 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git
In-Reply-To: <20071209183856.GA5587@bitplanet.net>

Kristian Høgsberg <krh@bitplanet.net> writes:

> The code in git to read the info in .git/branches is still there,
> but nothing ever writes to this, so lets stop creating it.
>
> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> ---
>
> As far as I can see this should be safe, but I admit to never really
> knowing what .git/branches was originally used for - tracking remote
> branches or something?  In any case, we only ever read from this dir
> so the only left in git to deal with this seems to be for compatibilty
> with older repos.

This was done purely so that Cogito would not barf on a repository
initialized with "git init" even if it did not bother creating necessary
leading directories when it writes its rough equivalent to .git/remotes/
information.  I do not recall if Cogito had such a problem anymore, but
I think it can safely go when Cogito is effectively dead.

^ permalink raw reply

* Re: [PATCH 3/4] Move the file read logic to read_patch_file() in builtin-apply.c
From: Junio C Hamano @ 2007-12-09 18:27 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <1197219900-19334-3-git-send-email-mh@glandium.org>

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

^ permalink raw reply

* Re: Something is broken in repack
From: Jon Smirl @ 2007-12-09 18:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, Git Mailing List
In-Reply-To: <7vprxgs36w.fsf@gitster.siamese.dyndns.org>

On 12/9/07, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Nicolas Pitre <nico@cam.org> writes:
> >
> >> On Fri, 7 Dec 2007, Jon Smirl wrote:
> >>
> >>> Starting with a 2GB pack of the same data my process size only grew to
> >>> 3GB with 2GB of mmaps.
> >>
> >> Which is quite reasonable, even if the same issue might still be there.
> >>
> >> So the problem seems to be related to the pack access code and not the
> >> repack code.  And it must have something to do with the number of deltas
> >> being replayed.  And because the repack is attempting delta compression
> >> roughly from newest to oldest, and because old objects are typically in
> >> a deeper delta chain, then this might explain the logarithmic slowdown.
> >>
> >> So something must be wrong with the delta cache in sha1_file.c somehow.
> >
> > I was reaching the same conclusion but haven't managed to spot anything
> > blatantly wrong in that area.  Will need to dig more.
>
> Does this problem have correlation with the use of threads?  Do you see
> the same bloat with or without THREADED_DELTA_SEARCH defined?
>

Something else seems to be wrong.

With threading turned off,  5000 CPU seconds and 13% done.
With threading turned on, threads = 1, 5000 CPU seconds, 13%
With threading turned on, threads = 2, 180 CPU seconds, 13%
With threading turned on, threads = 4, 150 CPU seconds, 13%

This can't be right, four cores are not 40x one core. So maybe the
observed logarithmic slow down is because the percent complete is
being reported wrong in the threaded case. If that's the case we may
be looking in the wrong place for problems.

The times are only approximate, I'm using the CPU for other things.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH 2/4] Use strbuf in http code
From: Mike Hommey @ 2007-12-09 18:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nick Hengeveld
In-Reply-To: <7vy7c3pwek.fsf@gitster.siamese.dyndns.org>

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

^ permalink raw reply

* Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary
From: David Kastrup @ 2007-12-09 18:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Mike Hommey, git
In-Reply-To: <7v63z7rb87.fsf@gitster.siamese.dyndns.org>

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

^ permalink raw reply

* Re: [PATCH 1/4] Cleanup variables in http.[ch]
From: Junio C Hamano @ 2007-12-09 18:21 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git
In-Reply-To: <1197219900-19334-1-git-send-email-mh@glandium.org>

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.

^ permalink raw reply

* Re: [PATCH] don't mention index refreshing side effect in git-status docs
From: Junio C Hamano @ 2007-12-09 18:15 UTC (permalink / raw)
  To: Jonathan del Strother; +Cc: git
In-Reply-To: <57518fd10712090816pa43cf97mbc1b8f5fbe81c66f@mail.gmail.com>

"Jonathan del Strother" <maillist@steelskies.com> writes:

> ... the GIT-VERSION-GEN
> script occasionally gives me spurious 'dirty' results (eg when a file
> timestamp changes despite the content having changed).

Good eyes.  Either refreshing before diff-index or using the "git diff"
Porcelain would be more appropriate.

^ permalink raw reply

* Re: [PATCH] Restore ls-remote reference pattern matching
From: Junio C Hamano @ 2007-12-09 18:15 UTC (permalink / raw)
  To: Sergey Vlasov; +Cc: git, Eyvind Bernhardsen
In-Reply-To: <20071209162632.a16bfd6e.vsu@altlinux.ru>

Sergey Vlasov <vsu@altlinux.ru> writes:

> This still does not match the behavior of the old shell implementation
> completely - because $pat was not quoted, shell pattern characters in
> $pat worked, and things like "git ls-remote . 'refs/heads/something--*'"
> were possible (and used in some of my scripts), so a full fnmatch()
> call is still needed.

Ah, true.  I wanted to cheat but that was a mistake.

^ permalink raw reply

* Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.
From: Junio C Hamano @ 2007-12-09 18:15 UTC (permalink / raw)
  To: Jim Meyering; +Cc: git list, Brian Gernhardt
In-Reply-To: <87hcitgpca.fsf@rho.meyering.net>

Jim Meyering <jim@meyering.net> writes:

> * config.c (store_write_pair): Don't read value[-1].
>
> Signed-off-by: Jim Meyering <meyering@redhat.com>
> ---
>  config.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/config.c b/config.c
> index ed96213..6031b38 100644
> --- a/config.c
> +++ b/config.c
> @@ -652,7 +652,7 @@ static int store_write_pair(int fd, const char* key, const char* value)
>  	for (i = 0; value[i]; i++)
>  		if (value[i] == ';' || value[i] == '#')
>  			quote = 1;
> -	if (value[i-1] == ' ')
> +	if (i && value[i-1] == ' ')
>  		quote = 1;
>
>  	if (write_in_full(fd, "\t", 1) != 1 ||

Thanks.

The fix may be correct but the comment above the part the patch fixes is
very misleading and I wasted a few minutes checking to see what it is
doing is correct.

The "quote" variable does not really control quoting; the quoting with
backslash is always done for the value, and this check is about adding a
dq pair around it, so that the parser does not lose leading or trailing
SP or string after start-of-comment marker characters.

^ permalink raw reply

* Re: [PATCH 2/4] Use strbuf in http code
From: Junio C Hamano @ 2007-12-09 18:15 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, Nick Hengeveld
In-Reply-To: <1197219900-19334-2-git-send-email-mh@glandium.org>

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.

^ permalink raw reply

* Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary
From: Mike Hommey @ 2007-12-09 18:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, David Kastrup
In-Reply-To: <7v63z7rb87.fsf@gitster.siamese.dyndns.org>

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

^ permalink raw reply

* Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary
From: Junio C Hamano @ 2007-12-09 18:09 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, David Kastrup
In-Reply-To: <1197220648-20433-1-git-send-email-mh@glandium.org>

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

^ permalink raw reply

* [PATCH] Remove .git/branches from the .git template.
From: Kristian Høgsberg @ 2007-12-09 18:38 UTC (permalink / raw)
  To: gitster; +Cc: git

The code in git to read the info in .git/branches is still there,
but nothing ever writes to this, so lets stop creating it.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---

As far as I can see this should be safe, but I admit to never really
knowing what .git/branches was originally used for - tracking remote
branches or something?  In any case, we only ever read from this dir
so the only left in git to deal with this seems to be for compatibilty
with older repos.

Kristian

 templates/branches-- |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
 delete mode 100644 templates/branches--

diff --git a/templates/branches-- b/templates/branches--
deleted file mode 100644
index fae8870..0000000
--- a/templates/branches--
+++ /dev/null
@@ -1 +0,0 @@
-: this is just to ensure the directory exists.
-- 
1.5.3.4

^ permalink raw reply related

* Re: [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary
From: Mike Hommey @ 2007-12-09 17:38 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1197220648-20433-1-git-send-email-mh@glandium.org>

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

^ permalink raw reply

* [PATCH] git-send-email.perl: Really add angle brackets to In-Reply-To if necessary
From: Mike Hommey @ 2007-12-09 17:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1197219900-19334-1-git-send-email-mh@glandium.org>

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

^ permalink raw reply related

* [PATCH 4/4] Add support for URLs to git-apply
From: Mike Hommey @ 2007-12-09 17:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1197219900-19334-3-git-send-email-mh@glandium.org>

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

^ permalink raw reply related

* [PATCH 1/4] Cleanup variables in http.[ch]
From: Mike Hommey @ 2007-12-09 17:04 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox