Git development
 help / color / mirror / Atom feed
* Re: [PATCH/Test] Build in merge is broken
From: Miklos Vajna @ 2008-07-13 17:46 UTC (permalink / raw)
  To: Sverre Hvammen Johansen; +Cc: git, Junio C Hamano
In-Reply-To: <20080713124100.GB10347@genesis.frugalware.org>

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

On Sun, Jul 13, 2008 at 02:41:00PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> > The test case when run will record the parents that were asked for which is
> > fine.  However, only c2 is recorded as a parent in the commit object.  Both
> > c1 and c2 should have been recorded.  The merge is otherwise working
> > correctly.
> 
> Thanks for the testcase, I'm on it. ;-)

So far what I see is that the input for the reduce_heads() function is
(c1, c0, c2, c0, c1). The expected output would be (c1, c2), but the
actual output is c2. So I suspect the bug is not in builtin-merge.c
itself but in reduce_heads().

Hmm..

Adding Junio to Cc, who is the original author of reduce_heads().

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

^ permalink raw reply

* Re: ! [PATCH/RFC] git-mailinfo: use strbuf's instead of fixed buffers
From: Lukas Sandström @ 2008-07-13 18:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vy747fx9x.fsf_-_@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Lukas Sandström <lukass@etek.chalmers.se> writes:
> 
>> -static char *sanity_check(char *name, char *email)
>> +static void sanity_check(struct strbuf *out, struct strbuf *name, struct strbuf *email)
>>  {
>> -	int len = strlen(name);
>> -	if (len < 3 || len > 60)
>> -		return email;
>> -	if (strchr(name, '@') || strchr(name, '<') || strchr(name, '>'))
>> -		return email;
>> -	return name;
>> +	struct strbuf o = STRBUF_INIT;
>> +	if (name->len < 3 || name->len > 60)
>> +		strbuf_addbuf(&o, email);
>> +	if (strchr(name->buf, '@') || strchr(name->buf, '<') ||
>> +		strchr(name->buf, '>'))
>> +		strbuf_addbuf(&o, email);
>> +	strbuf_addbuf(&o, name);
>> +	strbuf_reset(out);
>> +	strbuf_addbuf(out, &o);
>> +	strbuf_release(&o);
> 
> This does not look like a correct conversion.  When name is too short or
> too long, we do not even look at name and return email straight.  Perhaps
> this would be more faithful conversion:
> 
> 	struct strbuf *src = name;
> 	if (name->len < 3 ||
>             60 < name->len ||
> 	    strchr(name->buf, '@') ||
> 	    strchr(name->buf, '<') ||
> 	    strchr(name->buf, '>'))
> 		src = email;
> 	else if (name == out)
>         	return;
> 	strbuf_reset(out);
> 	strbuf_addbuf(out, src);
> 
> It is not your fault, but sanity_check() is a grave misnomer for this
> function.  This does "get_sane_name" (i.e. we have name and email but if
> name does not look right, use email instead).

Right. I changed the name and used your implementation.

>> -static int bogus_from(char *line)
>> +static int bogus_from(const struct strbuf *line)
>>  {

[ ... ]

>>  	return 1;
>>  }
> 
> Conversion looks correct but its return value does not make much sense
> (again, not your fault).  bogus_from() is given a bogus looking from line
> (it is not about checking if it is bogus), and returns 0 if we already
> have e-mail address, if the from line does not have bra-ket for grabbing
> e-mail address for, but returns 1 if we managed to get name and email
> pairs.  The inconsistency does not matter only because its sole caller
> handle_from() returns its return value, and its caller discards it.  We
> may be better off declaring this function and handle_from() as void.
> 

Done.

>>  	/* The remainder is name.  It could be "John Doe <john.doe@xz>"
>>  	 * or "john.doe@xz (John Doe)", but we have whited out the
>>  	 * email part, so trim from both ends, possibly removing
>>  	 * the () pair at the end.
>>  	 */
> 
> Now, it should read "but we have removed the email part", I think.

Done.
>> +	strbuf_trim(from);
>> +	if (*from->buf == '(')
>> +		strbuf_remove(&name, 0, 1);
>> +	if (*(from->buf + from->len - 1) == ')')
> 
> Can from be empty at this point before this check?
> 

Yes. Added a test for that.

>> +		strbuf_setlen(from, from->len - 1);
>> +
>> +	sanity_check(&name, from, &email);
>>  	return 1;
>>  }
> 
> We used to copy the data from the argument (in_line) before munging it in
> this function, but now we are modifying it in place (from).  Does this
> upset our caller, or the original code was just doing an extra unnecessary
> copy?

No, it's ok with the current code, since when handle_from() is called, it's
the last time we look at the header[i] field. I changed it to copy its argument
anyway, for future-proofing.

> 
>> -static int handle_header(char *line, char *data, int ofs)
>> +static void handle_header(struct strbuf **out, const struct strbuf *line)
>>  {
>> -	if (!line || !data)
>> -		return 1;
>> -
>> -	strcpy(data, line+ofs);
>> +	if (!*out) {
>> +		*out = xmalloc(sizeof(struct strbuf));
>> +		strbuf_init(*out, line->len);
>> +	} else
>> +		strbuf_reset(*out);
>>  
>> -	return 0;
>> +	strbuf_addbuf(*out, (struct strbuf *)line); /* const warning */
>>  }
> 
> I think its second parameter can safely become "const struct strbuf *";
> perhaps we should fix the definition of strbuf_addbuf() in your first
> patch?
> 
Done.

>> @@ -173,180 +153,176 @@ static int slurp_attr(const char *line, const char *name, char *attr)
>>  	else
>>  		ends = "; \t";
>>  	sz = strcspn(ap, ends);
>> -	memcpy(attr, ap, sz);
>> -	attr[sz] = 0;
>> +	strbuf_add(attr, ap, sz);
>>  	return 1;
>>  }
>>  
>>  struct content_type {
>> -	char *boundary;
>> -	int boundary_len;
>> +	struct strbuf *boundary;
>>  };
>>  
>>  static struct content_type content[MAX_BOUNDARIES];
> 
> Wouldn't it make more sense to get rid of "struct content_type" altogether
> and use "struct strbuf *content[MAX_BOUNDARIES]" directly?

Sure.

I'll send a patch updated with your comments shortly.

/Lukas

^ permalink raw reply

* [PATCH] Make some strbuf_*() struct strbuf arguments const.
From: Lukas Sandström @ 2008-07-13 18:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lukas Sandström, Git Mailing List
In-Reply-To: <487A46C5.6000503@etek.chalmers.se>

Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
---

 strbuf.c |    2 +-
 strbuf.h |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index 4aed752..7767170 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -67,7 +67,7 @@ void strbuf_rtrim(struct strbuf *sb)
 	sb->buf[sb->len] = '\0';
 }
 
-int strbuf_cmp(struct strbuf *a, struct strbuf *b)
+int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
 {
 	int cmp;
 	if (a->len < b->len) {
diff --git a/strbuf.h b/strbuf.h
index faec229..a1b0143 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -61,7 +61,7 @@ static inline void strbuf_swap(struct strbuf *a, struct strbuf *b) {
 }
 
 /*----- strbuf size related -----*/
-static inline size_t strbuf_avail(struct strbuf *sb) {
+static inline size_t strbuf_avail(const struct strbuf *sb) {
 	return sb->alloc ? sb->alloc - sb->len - 1 : 0;
 }
 
@@ -78,7 +78,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
 
 /*----- content related -----*/
 extern void strbuf_rtrim(struct strbuf *);
-extern int strbuf_cmp(struct strbuf *, struct strbuf *);
+extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
 
 /*----- add data in your buffer -----*/
 static inline void strbuf_addch(struct strbuf *sb, int c) {
@@ -98,7 +98,7 @@ extern void strbuf_add(struct strbuf *, const void *, size_t);
 static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
 	strbuf_add(sb, s, strlen(s));
 }
-static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
+static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2) {
 	strbuf_add(sb, sb2->buf, sb2->len);
 }
 extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);
-- 
1.5.4.5

^ permalink raw reply related

* [PATCH] Add some useful functions for strbuf manipulation.
From: Lukas Sandström @ 2008-07-13 18:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lukas Sandström, Git Mailing List
In-Reply-To: <487A4948.8080003@etek.chalmers.se>

Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
---
 strbuf.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 strbuf.h |    6 +++++
 2 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index 7767170..6294940 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -60,6 +60,18 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
 	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
 }
 
+void strbuf_trim(struct strbuf *sb)
+{
+	char *b = sb->buf;
+	while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
+		sb->len--;
+	while(sb->len > 0 && isspace(*b)) {
+		b++;
+		sb->len--;
+	}
+	memmove(sb->buf, b, sb->len);
+	sb->buf[sb->len] = '\0';
+}
 void strbuf_rtrim(struct strbuf *sb)
 {
 	while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
@@ -67,6 +79,64 @@ void strbuf_rtrim(struct strbuf *sb)
 	sb->buf[sb->len] = '\0';
 }
 
+void strbuf_ltrim(struct strbuf *sb)
+{
+	char *b = sb->buf;
+	while(sb->len > 0 && isspace(*b)) {
+		b++;
+		sb->len--;
+	}
+	memmove(sb->buf, b, sb->len);
+	sb->buf[sb->len] = '\0';
+}
+
+void strbuf_tolower(struct strbuf *sb)
+{
+	int i;
+	for (i = 0; i < sb->len; i++)
+		sb->buf[i] = tolower(sb->buf[i]);
+}
+
+struct strbuf ** strbuf_split(const struct strbuf *sb, int delim)
+{
+	int alloc = 2, pos = 0;
+	char *n, *p;
+	struct strbuf **ret;
+	struct strbuf *t;
+
+	ret = xcalloc(alloc, sizeof(struct strbuf *));
+	p = n = sb->buf;
+	while (n < sb->buf + sb->len) {
+		int len;
+		n = memchr(n, delim, sb->len - (n - sb->buf));
+		if (pos + 1 >= alloc) {
+			alloc = alloc * 2;
+			ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
+		}
+		if (!n)
+			n = sb->buf + sb->len - 1;
+		len = n - p + 1;
+		t = xmalloc(sizeof(struct strbuf));
+		strbuf_init(t, len);
+		strbuf_add(t, p, len);
+		ret[pos] = t;
+		ret[++pos] = NULL;
+		p = ++n;
+	}
+	return ret;
+}
+
+void strbuf_list_free(struct strbuf ** sbs)
+{
+	struct strbuf **s = sbs;
+
+	while(*s) {
+		strbuf_release(*s);
+		free(*s++);
+	}
+	free(sbs);
+}
+
 int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
 {
 	int cmp;
diff --git a/strbuf.h b/strbuf.h
index a1b0143..5e65a3f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -77,8 +77,14 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
 #define strbuf_reset(sb)  strbuf_setlen(sb, 0)
 
 /*----- content related -----*/
+extern void strbuf_trim(struct strbuf *);
 extern void strbuf_rtrim(struct strbuf *);
+extern void strbuf_ltrim(struct strbuf *);
 extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
+extern void strbuf_tolower(struct strbuf *);
+
+extern struct strbuf ** strbuf_split(const struct strbuf*, int delim);
+extern void strbuf_list_free(struct strbuf **);
 
 /*----- add data in your buffer -----*/
 static inline void strbuf_addch(struct strbuf *sb, int c) {
-- 
1.5.4.5

^ permalink raw reply related

* [PATCH] git-mailinfo: use strbuf's instead of fixed buffers
From: Lukas Sandström @ 2008-07-13 18:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lukas Sandström, Git Mailing List
In-Reply-To: <487A497E.6030308@etek.chalmers.se>

Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
---
 builtin-mailinfo.c |  750 ++++++++++++++++++++++++----------------------------
 1 files changed, 348 insertions(+), 402 deletions(-)

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 2d1520f..329ac43 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -5,14 +5,15 @@
 #include "cache.h"
 #include "builtin.h"
 #include "utf8.h"
+#include "strbuf.h"
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
 static int keep_subject;
 static const char *metainfo_charset;
-static char line[1000];
-static char name[1000];
-static char email[1000];
+static struct strbuf line = STRBUF_INIT;
+static struct strbuf name = STRBUF_INIT;
+static struct strbuf email = STRBUF_INIT;
 
 static enum  {
 	TE_DONTCARE, TE_QP, TE_BASE64,
@@ -21,74 +22,77 @@ static enum  {
 	TYPE_TEXT, TYPE_OTHER,
 } message_type;
 
-static char charset[256];
+static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
-static char **p_hdr_data, **s_hdr_data;
+static struct strbuf **p_hdr_data, **s_hdr_data;
 
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
-static char *sanity_check(char *name, char *email)
+static void get_sane_name(struct strbuf *out, struct strbuf *name, struct strbuf *email)
 {
-	int len = strlen(name);
-	if (len < 3 || len > 60)
-		return email;
-	if (strchr(name, '@') || strchr(name, '<') || strchr(name, '>'))
-		return email;
-	return name;
+	struct strbuf *src = name;
+	if (name->len < 3 || 60 < name->len || strchr(name->buf, '@') ||
+		strchr(name->buf, '<') || strchr(name->buf, '>'))
+		src = email;
+	else if (name == out)
+		return;
+	strbuf_reset(out);
+	strbuf_addbuf(out, src);
 }
 
-static int bogus_from(char *line)
+static void parse_bogus_from(const struct strbuf *line)
 {
 	/* John Doe <johndoe> */
-	char *bra, *ket, *dst, *cp;
 
+	char *bra, *ket;
 	/* This is fallback, so do not bother if we already have an
 	 * e-mail address.
 	 */
-	if (*email)
-		return 0;
+	if (email.len)
+		return;
 
-	bra = strchr(line, '<');
+	bra = strchr(line->buf, '<');
 	if (!bra)
-		return 0;
+		return;
 	ket = strchr(bra, '>');
 	if (!ket)
-		return 0;
+		return;
 
-	for (dst = email, cp = bra+1; cp < ket; )
-		*dst++ = *cp++;
-	*dst = 0;
-	for (cp = line; isspace(*cp); cp++)
-		;
-	for (bra--; isspace(*bra); bra--)
-		*bra = 0;
-	cp = sanity_check(cp, email);
-	strcpy(name, cp);
-	return 1;
+	strbuf_reset(&email);
+	strbuf_add(&email, bra + 1, ket - bra - 1);
+
+	strbuf_reset(&name);
+	strbuf_add(&name, line->buf, bra - line->buf);
+	strbuf_trim(&name);
+	get_sane_name(&name, &name, &email);
 }
 
-static int handle_from(char *in_line)
+static void handle_from(const struct strbuf *from)
 {
-	char line[1000];
 	char *at;
-	char *dst;
+	size_t el;
+	struct strbuf f;
 
-	strcpy(line, in_line);
-	at = strchr(line, '@');
+	strbuf_init(&f, from->len);
+	strbuf_addbuf(&f, from);
+
+	at = strchr(f.buf, '@');
 	if (!at)
-		return bogus_from(line);
+		return parse_bogus_from(from);
 
 	/*
 	 * If we already have one email, don't take any confusing lines
 	 */
-	if (*email && strchr(at+1, '@'))
-		return 0;
+	if (email.len && strchr(at + 1, '@')) {
+		strbuf_release(&f);
+		return;
+	}
 
 	/* Pick up the string around '@', possibly delimited with <>
-	 * pair; that is the email part.  White them out while copying.
+	 * pair; that is the email part.
 	 */
-	while (at > line) {
+	while (at > f.buf) {
 		char c = at[-1];
 		if (isspace(c))
 			break;
@@ -98,56 +102,35 @@ static int handle_from(char *in_line)
 		}
 		at--;
 	}
-	dst = email;
-	for (;;) {
-		unsigned char c = *at;
-		if (!c || c == '>' || isspace(c)) {
-			if (c == '>')
-				*at = ' ';
-			break;
-		}
-		*at++ = ' ';
-		*dst++ = c;
-	}
-	*dst++ = 0;
+	el = strcspn(at, " \n\t\r\v\f>");
+	strbuf_reset(&email);
+	strbuf_add(&email, at, el);
+	strbuf_remove(&f, at - f.buf, el + 1);
 
 	/* The remainder is name.  It could be "John Doe <john.doe@xz>"
-	 * or "john.doe@xz (John Doe)", but we have whited out the
+	 * or "john.doe@xz (John Doe)", but we have removed the
 	 * email part, so trim from both ends, possibly removing
 	 * the () pair at the end.
 	 */
-	at = line + strlen(line);
-	while (at > line) {
-		unsigned char c = *--at;
-		if (!isspace(c)) {
-			at[(c == ')') ? 0 : 1] = 0;
-			break;
-		}
-	}
+	strbuf_trim(&f);
+	if (f.buf[0] == '(')
+		strbuf_remove(&name, 0, 1);
+	if (f.len && f.buf[f.len - 1] == ')')
+		strbuf_setlen(&f, f.len - 1);
 
-	at = line;
-	for (;;) {
-		unsigned char c = *at;
-		if (!c || !isspace(c)) {
-			if (c == '(')
-				at++;
-			break;
-		}
-		at++;
-	}
-	at = sanity_check(at, email);
-	strcpy(name, at);
-	return 1;
+	get_sane_name(&name, &f, &email);
+	strbuf_release(&f);
 }
 
-static int handle_header(char *line, char *data, int ofs)
+static void handle_header(struct strbuf **out, const struct strbuf *line)
 {
-	if (!line || !data)
-		return 1;
-
-	strcpy(data, line+ofs);
+	if (!*out) {
+		*out = xmalloc(sizeof(struct strbuf));
+		strbuf_init(*out, line->len);
+	} else
+		strbuf_reset(*out);
 
-	return 0;
+	strbuf_addbuf(*out, line);
 }
 
 /* NOTE NOTE NOTE.  We do not claim we do full MIME.  We just attempt
@@ -156,13 +139,13 @@ static int handle_header(char *line, char *data, int ofs)
  * case insensitively.
  */
 
-static int slurp_attr(const char *line, const char *name, char *attr)
+static int slurp_attr(const char *line, const char *name, struct strbuf *attr)
 {
 	const char *ends, *ap = strcasestr(line, name);
 	size_t sz;
 
 	if (!ap) {
-		*attr = 0;
+		strbuf_setlen(attr, 0);
 		return 0;
 	}
 	ap += strlen(name);
@@ -173,180 +156,171 @@ static int slurp_attr(const char *line, const char *name, char *attr)
 	else
 		ends = "; \t";
 	sz = strcspn(ap, ends);
-	memcpy(attr, ap, sz);
-	attr[sz] = 0;
+	strbuf_add(attr, ap, sz);
 	return 1;
 }
 
-struct content_type {
-	char *boundary;
-	int boundary_len;
-};
-
-static struct content_type content[MAX_BOUNDARIES];
+static struct strbuf *content[MAX_BOUNDARIES];
 
-static struct content_type *content_top = content;
+static struct strbuf **content_top = content;
 
-static int handle_content_type(char *line)
+static void handle_content_type(struct strbuf *line)
 {
-	char boundary[256];
+	struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
+	strbuf_init(boundary, line->len);
 
-	if (strcasestr(line, "text/") == NULL)
+	if (!strcasestr(line->buf, "text/"))
 		 message_type = TYPE_OTHER;
-	if (slurp_attr(line, "boundary=", boundary + 2)) {
-		memcpy(boundary, "--", 2);
+	if (slurp_attr(line->buf, "boundary=", boundary)) {
+		strbuf_insert(boundary, 0, "--", 2);
 		if (content_top++ >= &content[MAX_BOUNDARIES]) {
 			fprintf(stderr, "Too many boundaries to handle\n");
 			exit(1);
 		}
-		content_top->boundary_len = strlen(boundary);
-		content_top->boundary = xmalloc(content_top->boundary_len+1);
-		strcpy(content_top->boundary, boundary);
+		*content_top = boundary;
+		boundary = NULL;
 	}
-	if (slurp_attr(line, "charset=", charset)) {
-		int i, c;
-		for (i = 0; (c = charset[i]) != 0; i++)
-			charset[i] = tolower(c);
+	if (slurp_attr(line->buf, "charset=", &charset))
+		strbuf_tolower(&charset);
+
+	if (boundary) {
+		strbuf_release(boundary);
+		free(boundary);
 	}
-	return 0;
 }
 
-static int handle_content_transfer_encoding(char *line)
+static void handle_content_transfer_encoding(const struct strbuf *line)
 {
-	if (strcasestr(line, "base64"))
+	if (strcasestr(line->buf, "base64"))
 		transfer_encoding = TE_BASE64;
-	else if (strcasestr(line, "quoted-printable"))
+	else if (strcasestr(line->buf, "quoted-printable"))
 		transfer_encoding = TE_QP;
 	else
 		transfer_encoding = TE_DONTCARE;
-	return 0;
-}
-
-static int is_multipart_boundary(const char *line)
-{
-	return (!memcmp(line, content_top->boundary, content_top->boundary_len));
 }
 
-static int eatspace(char *line)
+static int is_multipart_boundary(const struct strbuf *line)
 {
-	int len = strlen(line);
-	while (len > 0 && isspace(line[len-1]))
-		line[--len] = 0;
-	return len;
+	return !strbuf_cmp(line, *content_top);
 }
 
-static char *cleanup_subject(char *subject)
+static void cleanup_subject(struct strbuf *subject)
 {
-	for (;;) {
-		char *p;
-		int len, remove;
-		switch (*subject) {
+	char *pos;
+	size_t remove;
+	while (subject->len) {
+		switch (*subject->buf) {
 		case 'r': case 'R':
-			if (!memcmp("e:", subject+1, 2)) {
-				subject += 3;
+			if (subject->len <= 3)
+				break;
+			if (!memcmp(subject->buf + 1, "e:", 2)) {
+				strbuf_remove(subject, 0, 3);
 				continue;
 			}
 			break;
 		case ' ': case '\t': case ':':
-			subject++;
+			strbuf_remove(subject, 0, 1);
 			continue;
-
 		case '[':
-			p = strchr(subject, ']');
-			if (!p) {
-				subject++;
-				continue;
-			}
-			len = strlen(p);
-			remove = p - subject;
-			if (remove <= len *2) {
-				subject = p+1;
-				continue;
-			}
+			if ((pos = strchr(subject->buf, ']'))) {
+				remove = pos - subject->buf + 1;
+				/* Don't remove too much. */
+				if (remove <= (subject->len - remove + 1) * 2) {
+					strbuf_remove(subject, 0, remove);
+					continue;
+				}
+			} else
+				strbuf_remove(subject, 0, 1);
 			break;
 		}
-		eatspace(subject);
-		return subject;
+		strbuf_trim(subject);
+		return;
 	}
 }
 
-static void cleanup_space(char *buf)
+static void cleanup_space(struct strbuf *sb)
 {
-	unsigned char c;
-	while ((c = *buf) != 0) {
-		buf++;
-		if (isspace(c)) {
-			buf[-1] = ' ';
-			c = *buf;
-			while (isspace(c)) {
-				int len = strlen(buf);
-				memmove(buf, buf+1, len);
-				c = *buf;
-			}
+	size_t pos, cnt;
+	for (pos = 0; pos < sb->len; pos++) {
+		if (isspace(sb->buf[pos])) {
+			sb->buf[pos] = ' ';
+			for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
+			strbuf_remove(sb, pos + 1, cnt);
 		}
 	}
 }
 
-static void decode_header(char *it, unsigned itsize);
+static void decode_header(struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
 	"From","Subject","Date",
 };
 
-static int check_header(char *line, unsigned linesize, char **hdr_data, int overwrite)
+static inline int cmp_header(const struct strbuf *line, const char *hdr)
 {
-	int i;
+	int len = strlen(hdr);
+	return !strncasecmp(line->buf, hdr, len) && line->len > len &&
+			line->buf[len] == ':' && isspace(line->buf[len + 1]);
+}
 
+static int check_header(const struct strbuf *line,
+				struct strbuf *hdr_data[], int overwrite)
+{
+	int i, ret = 0, len;
+	struct strbuf sb = STRBUF_INIT;
 	/* search for the interesting parts */
 	for (i = 0; header[i]; i++) {
 		int len = strlen(header[i]);
-		if ((!hdr_data[i] || overwrite) &&
-		    !strncasecmp(line, header[i], len) &&
-		    line[len] == ':' && isspace(line[len + 1])) {
+		if ((!hdr_data[i] || overwrite) && cmp_header(line, header[i])) {
 			/* Unwrap inline B and Q encoding, and optionally
 			 * normalize the meta information to utf8.
 			 */
-			decode_header(line + len + 2, linesize - len - 2);
-			hdr_data[i] = xmalloc(1000 * sizeof(char));
-			if (! handle_header(line, hdr_data[i], len + 2)) {
-				return 1;
-			}
+			strbuf_add(&sb, line->buf + len + 2, line->len - len -2);
+			decode_header(&sb);
+			handle_header(&hdr_data[i], &sb);
+			ret = 1;
+			goto check_header_out;
 		}
 	}
 
 	/* Content stuff */
-	if (!strncasecmp(line, "Content-Type", 12) &&
-		line[12] == ':' && isspace(line[12 + 1])) {
-		decode_header(line + 12 + 2, linesize - 12 - 2);
-		if (! handle_content_type(line)) {
-			return 1;
-		}
-	}
-	if (!strncasecmp(line, "Content-Transfer-Encoding", 25) &&
-		line[25] == ':' && isspace(line[25 + 1])) {
-		decode_header(line + 25 + 2, linesize - 25 - 2);
-		if (! handle_content_transfer_encoding(line)) {
-			return 1;
-		}
+	if (cmp_header(line, "Content-Type")) {
+		len = strlen("Content-Type: ");
+		strbuf_add(&sb, line->buf + len, line->len - len);
+		decode_header(&sb);
+		strbuf_insert(&sb, 0, "Content-Type: ", len);
+		handle_content_type(&sb);
+		ret = 1;
+		goto check_header_out;
+	}
+	if (cmp_header(line, "Content-Transfer-Encoding")) {
+		len = strlen("Content-Transfer-Encoding: ");
+		strbuf_add(&sb, line->buf + len, line->len - len);
+		decode_header(&sb);
+		handle_content_transfer_encoding(&sb);
+		ret = 1;
+		goto check_header_out;
 	}
 
 	/* for inbody stuff */
-	if (!memcmp(">From", line, 5) && isspace(line[5]))
-		return 1;
-	if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
+	if (!prefixcmp(line->buf, ">From") && isspace(line->buf[5]))
+		ret = 1; /* Should this return 0? */
+		goto check_header_out;
+	if (!prefixcmp(line->buf, "[PATCH]") && isspace(line->buf[7])) {
 		for (i = 0; header[i]; i++) {
 			if (!memcmp("Subject", header[i], 7)) {
-				if (! handle_header(line, hdr_data[i], 0)) {
-					return 1;
-				}
+				handle_header(&hdr_data[i], line);
+				ret = 1;
+				goto check_header_out;
 			}
 		}
 	}
 
-	/* no match */
-	return 0;
+check_header_out:
+	strbuf_release(&sb);
+	return ret;
 }
 
-static int is_rfc2822_header(char *line)
+static int is_rfc2822_header(const struct strbuf *line)
 {
 	/*
 	 * The section that defines the loosest possible
@@ -357,15 +331,15 @@ static int is_rfc2822_header(char *line)
 	 * ftext = %d33-57 / %59-126
 	 */
 	int ch;
-	char *cp = line;
+	char *cp = line->buf;
 
 	/* Count mbox From headers as headers */
-	if (!memcmp(line, "From ", 5) || !memcmp(line, ">From ", 6))
+	if (!prefixcmp(cp, "From ") || !prefixcmp(cp, ">From "))
 		return 1;
 
 	while ((ch = *cp++)) {
 		if (ch == ':')
-			return cp != line;
+			return 1;
 		if ((33 <= ch && ch <= 57) ||
 		    (59 <= ch && ch <= 126))
 			continue;
@@ -374,34 +348,20 @@ static int is_rfc2822_header(char *line)
 	return 0;
 }
 
-/*
- * sz is size of 'line' buffer in bytes.  Must be reasonably
- * long enough to hold one physical real-world e-mail line.
- */
-static int read_one_header_line(char *line, int sz, FILE *in)
+static int read_one_header_line(struct strbuf *line, FILE *in)
 {
-	int len;
-
-	/*
-	 * We will read at most (sz-1) bytes and then potentially
-	 * re-add NUL after it.  Accessing line[sz] after this is safe
-	 * and we can allow len to grow up to and including sz.
-	 */
-	sz--;
-
 	/* Get the first part of the line. */
-	if (!fgets(line, sz, in))
+	if (strbuf_getline(line, in, '\n'))
 		return 0;
 
 	/*
 	 * Is it an empty line or not a valid rfc2822 header?
 	 * If so, stop here, and return false ("not a header")
 	 */
-	len = eatspace(line);
-	if (!len || !is_rfc2822_header(line)) {
+	strbuf_rtrim(line);
+	if (!line->len || !is_rfc2822_header(line)) {
 		/* Re-add the newline */
-		line[len] = '\n';
-		line[len + 1] = '\0';
+		strbuf_addch(line, '\n');
 		return 0;
 	}
 
@@ -410,65 +370,53 @@ static int read_one_header_line(char *line, int sz, FILE *in)
 	 * Yuck, 2822 header "folding"
 	 */
 	for (;;) {
-		int peek, addlen;
-		static char continuation[1000];
+		int peek;
+		struct strbuf continuation = STRBUF_INIT;
 
 		peek = fgetc(in); ungetc(peek, in);
 		if (peek != ' ' && peek != '\t')
 			break;
-		if (!fgets(continuation, sizeof(continuation), in))
+		if (strbuf_getline(&continuation, in, '\n'))
 			break;
-		addlen = eatspace(continuation);
-		if (len < sz - 1) {
-			if (addlen >= sz - len)
-				addlen = sz - len - 1;
-			memcpy(line + len, continuation, addlen);
-			line[len] = '\n';
-			len += addlen;
-		}
+		continuation.buf[0] = '\n';
+		strbuf_rtrim(&continuation);
+		strbuf_addbuf(line, &continuation);
 	}
-	line[len] = 0;
 
 	return 1;
 }
 
-static int decode_q_segment(char *in, char *ot, unsigned otsize, char *ep, int rfc2047)
+static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 {
-	char *otbegin = ot;
-	char *otend = ot + otsize;
+	const char *in = q_seg->buf;
 	int c;
-	while ((c = *in++) != 0 && (in <= ep)) {
-		if (ot == otend) {
-			*--ot = '\0';
-			return -1;
-		}
+	struct strbuf *out = xmalloc(sizeof(struct strbuf));
+	strbuf_init(out, q_seg->len);
+
+	while ((c = *in++) != 0) {
 		if (c == '=') {
 			int d = *in++;
 			if (d == '\n' || !d)
 				break; /* drop trailing newline */
-			*ot++ = ((hexval(d) << 4) | hexval(*in++));
+			strbuf_addch(out, (hexval(d) << 4) | hexval(*in++));
 			continue;
 		}
 		if (rfc2047 && c == '_') /* rfc2047 4.2 (2) */
 			c = 0x20;
-		*ot++ = c;
+		strbuf_addch(out, c);
 	}
-	*ot = 0;
-	return (ot - otbegin);
+	return out;
 }
 
-static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
+static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
 {
 	/* Decode in..ep, possibly in-place to ot */
 	int c, pos = 0, acc = 0;
-	char *otbegin = ot;
-	char *otend = ot + otsize;
+	const char *in = b_seg->buf;
+	struct strbuf *out = xmalloc(sizeof(struct strbuf));
+	strbuf_init(out, b_seg->len);
 
-	while ((c = *in++) != 0 && (in <= ep)) {
-		if (ot == otend) {
-			*--ot = '\0';
-			return -1;
-		}
+	while ((c = *in++) != 0) {
 		if (c == '+')
 			c = 62;
 		else if (c == '/')
@@ -493,21 +441,20 @@ static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
 			acc = (c << 2);
 			break;
 		case 1:
-			*ot++ = (acc | (c >> 4));
+			strbuf_addch(out, (acc | (c >> 4)));
 			acc = (c & 15) << 4;
 			break;
 		case 2:
-			*ot++ = (acc | (c >> 2));
+			strbuf_addch(out, (acc | (c >> 2)));
 			acc = (c & 3) << 6;
 			break;
 		case 3:
-			*ot++ = (acc | c);
+			strbuf_addch(out, (acc | c));
 			acc = pos = 0;
 			break;
 		}
 	}
-	*ot = 0;
-	return (ot - otbegin);
+	return out;
 }
 
 /*
@@ -521,16 +468,16 @@ static int decode_b_segment(char *in, char *ot, unsigned otsize, char *ep)
  * Otherwise, we default to assuming it is Latin1 for historical
  * reasons.
  */
-static const char *guess_charset(const char *line, const char *target_charset)
+static const char *guess_charset(const struct strbuf *line, const char *target_charset)
 {
 	if (is_encoding_utf8(target_charset)) {
-		if (is_utf8(line))
+		if (is_utf8(line->buf))
 			return NULL;
 	}
 	return "latin1";
 }
 
-static void convert_to_utf8(char *line, unsigned linesize, const char *charset)
+static void convert_to_utf8(struct strbuf *line, const char *charset)
 {
 	char *out;
 
@@ -542,112 +489,119 @@ static void convert_to_utf8(char *line, unsigned linesize, const char *charset)
 
 	if (!strcmp(metainfo_charset, charset))
 		return;
-	out = reencode_string(line, metainfo_charset, charset);
+	out = reencode_string(line->buf, metainfo_charset, charset);
 	if (!out)
 		die("cannot convert from %s to %s\n",
 		    charset, metainfo_charset);
-	strlcpy(line, out, linesize);
-	free(out);
+	strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static int decode_header_bq(char *it, unsigned itsize)
+static int decode_header_bq(struct strbuf *it)
 {
 	char *in, *out, *ep, *cp, *sp;
-	char outbuf[1000];
+	struct strbuf outbuf = STRBUF_INIT, *dec;
+	struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
 	int rfc2047 = 0;
 
-	in = it;
-	out = outbuf;
-	while ((ep = strstr(in, "=?")) != NULL) {
-		int sz, encoding;
-		char charset_q[256], piecebuf[256];
+	in = it->buf;
+	while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
+		int encoding;
+		strbuf_reset(&charset_q);
+		strbuf_reset(&piecebuf);
 		rfc2047 = 1;
 
 		if (in != ep) {
-			sz = ep - in;
-			memcpy(out, in, sz);
-			out += sz;
-			in += sz;
+			strbuf_add(&outbuf, in, ep - in);
+			in = ep;
 		}
 		/* E.g.
 		 * ep : "=?iso-2022-jp?B?GyR...?= foo"
 		 * ep : "=?ISO-8859-1?Q?Foo=FCbar?= baz"
 		 */
 		ep += 2;
-		cp = strchr(ep, '?');
-		if (!cp)
-			return rfc2047; /* no munging */
-		for (sp = ep; sp < cp; sp++)
-			charset_q[sp - ep] = tolower(*sp);
-		charset_q[cp - ep] = 0;
+
+		if (ep - it->buf >= it->len || !(cp = strchr(ep, '?')))
+			goto decode_header_bq_out;
+
+		if (cp + 3 - it->buf > it->len)
+			goto decode_header_bq_out;
+		strbuf_add(&charset_q, ep, cp - ep);
+		strbuf_tolower(&charset_q);
+
 		encoding = cp[1];
 		if (!encoding || cp[2] != '?')
-			return rfc2047; /* no munging */
+			goto decode_header_bq_out;
 		ep = strstr(cp + 3, "?=");
 		if (!ep)
-			return rfc2047; /* no munging */
+			goto decode_header_bq_out;
+		strbuf_add(&piecebuf, cp + 3, ep - cp - 3);
 		switch (tolower(encoding)) {
 		default:
-			return rfc2047; /* no munging */
+			goto decode_header_bq_out;
 		case 'b':
-			sz = decode_b_segment(cp + 3, piecebuf, sizeof(piecebuf), ep);
+			dec = decode_b_segment(&piecebuf);
 			break;
 		case 'q':
-			sz = decode_q_segment(cp + 3, piecebuf, sizeof(piecebuf), ep, 1);
+			dec = decode_q_segment(&piecebuf, 1);
 			break;
 		}
-		if (sz < 0)
-			return rfc2047;
 		if (metainfo_charset)
-			convert_to_utf8(piecebuf, sizeof(piecebuf), charset_q);
+			convert_to_utf8(dec, charset_q.buf);
 
-		sz = strlen(piecebuf);
-		if (outbuf + sizeof(outbuf) <= out + sz)
-			return rfc2047; /* no munging */
-		strcpy(out, piecebuf);
-		out += sz;
+		strbuf_addbuf(&outbuf, dec);
+		strbuf_release(dec);
+		free(dec);
 		in = ep + 2;
 	}
-	strcpy(out, in);
-	strlcpy(it, outbuf, itsize);
+	strbuf_addstr(&outbuf, in);
+	strbuf_reset(it);
+	strbuf_addbuf(it, &outbuf);
+decode_header_bq_out:
+	strbuf_release(&outbuf);
+	strbuf_release(&charset_q);
+	strbuf_release(&piecebuf);
 	return rfc2047;
 }
 
-static void decode_header(char *it, unsigned itsize)
+static void decode_header(struct strbuf *it)
 {
-
-	if (decode_header_bq(it, itsize))
+	if (decode_header_bq(it))
 		return;
 	/* otherwise "it" is a straight copy of the input.
 	 * This can be binary guck but there is no charset specified.
 	 */
 	if (metainfo_charset)
-		convert_to_utf8(it, itsize, "");
+		convert_to_utf8(it, "");
 }
 
-static int decode_transfer_encoding(char *line, unsigned linesize, int inputlen)
+static void decode_transfer_encoding(struct strbuf *line)
 {
-	char *ep;
+	struct strbuf *ret;
+	int len;
 
 	switch (transfer_encoding) {
 	case TE_QP:
-		ep = line + inputlen;
-		return decode_q_segment(line, line, linesize, ep, 0);
+		ret = decode_q_segment(line, 0);
+		break;
 	case TE_BASE64:
-		ep = line + inputlen;
-		return decode_b_segment(line, line, linesize, ep);
+		ret = decode_b_segment(line);
+		break;
 	case TE_DONTCARE:
 	default:
-		return inputlen;
+		return;
 	}
+	strbuf_reset(line);
+	strbuf_addbuf(line, ret);
+	strbuf_release(ret);
+	free(ret);
 }
 
-static int handle_filter(char *line, unsigned linesize, int linelen);
+static void handle_filter(struct strbuf *line);
 
 static int find_boundary(void)
 {
-	while(fgets(line, sizeof(line), fin) != NULL) {
-		if (is_multipart_boundary(line))
+	while(!strbuf_getline(&line, fin, '\n')) {
+		if (is_multipart_boundary(&line))
 			return 1;
 	}
 	return 0;
@@ -655,12 +609,17 @@ static int find_boundary(void)
 
 static int handle_boundary(void)
 {
-	char newline[]="\n";
+	struct strbuf newline = STRBUF_INIT;
+
+	strbuf_addch(&newline, '\n');
 again:
-	if (!memcmp(line+content_top->boundary_len, "--", 2)) {
+	if (line.len >= (*content_top)->len + 2 &&
+	    !memcmp(line.buf + (*content_top)->len, "--", 2)) {
 		/* we hit an end boundary */
 		/* pop the current boundary off the stack */
-		free(content_top->boundary);
+		strbuf_release(*content_top);
+		free(*content_top);
+		*content_top = NULL;
 
 		/* technically won't happen as is_multipart_boundary()
 		   will fail first.  But just in case..
@@ -670,7 +629,8 @@ again:
 					"can't recover\n");
 			exit(1);
 		}
-		handle_filter(newline, sizeof(newline), strlen(newline));
+		handle_filter(&newline);
+		strbuf_release(&newline);
 
 		/* skip to the next boundary */
 		if (!find_boundary())
@@ -680,39 +640,44 @@ again:
 
 	/* set some defaults */
 	transfer_encoding = TE_DONTCARE;
-	charset[0] = 0;
+	strbuf_reset(&charset);
 	message_type = TYPE_TEXT;
 
 	/* slurp in this section's info */
-	while (read_one_header_line(line, sizeof(line), fin))
-		check_header(line, sizeof(line), p_hdr_data, 0);
+	while (read_one_header_line(&line, fin))
+		check_header(&line, p_hdr_data, 0);
 
+	strbuf_release(&newline);
 	/* eat the blank line after section info */
-	return (fgets(line, sizeof(line), fin) != NULL);
+	return (strbuf_getline(&line, fin, '\n') == 0);
 }
 
-static inline int patchbreak(const char *line)
+static inline int patchbreak(const struct strbuf *line)
 {
+	size_t i;
+
 	/* Beginning of a "diff -" header? */
-	if (!memcmp("diff -", line, 6))
+	if (!prefixcmp(line->buf, "diff -"))
 		return 1;
 
 	/* CVS "Index: " line? */
-	if (!memcmp("Index: ", line, 7))
+	if (!prefixcmp(line->buf, "Index: "))
 		return 1;
 
 	/*
 	 * "--- <filename>" starts patches without headers
 	 * "---<sp>*" is a manual separator
 	 */
-	if (!memcmp("---", line, 3)) {
-		line += 3;
+	if (line->len < 4)
+		return 0;
+
+	if (!prefixcmp(line->buf, "---")) {
 		/* space followed by a filename? */
-		if (line[0] == ' ' && !isspace(line[1]))
+		if (line->buf[3] == ' ' && !isspace(line->buf[4]))
 			return 1;
 		/* Just whitespace? */
-		for (;;) {
-			unsigned char c = *line++;
+		for (i = 3; i < line->len; i++) {
+			unsigned char c = line->buf[i];
 			if (c == '\n')
 				return 1;
 			if (!isspace(c))
@@ -723,32 +688,25 @@ static inline int patchbreak(const char *line)
 	return 0;
 }
 
-
-static int handle_commit_msg(char *line, unsigned linesize)
+static int handle_commit_msg(struct strbuf *line)
 {
 	static int still_looking = 1;
-	char *endline = line + linesize;
+	char *c;
 
 	if (!cmitmsg)
 		return 0;
 
 	if (still_looking) {
-		char *cp = line;
-		if (isspace(*line)) {
-			for (cp = line + 1; *cp; cp++) {
-				if (!isspace(*cp))
-					break;
-			}
-			if (!*cp)
-				return 0;
-		}
-		if ((still_looking = check_header(cp, endline - cp, s_hdr_data, 0)) != 0)
+		strbuf_ltrim(line);
+		if (!line->len)
+			return 0;
+		if ((still_looking = check_header(line, s_hdr_data, 0)) != 0)
 			return 0;
 	}
 
 	/* normalize the log message to UTF-8. */
 	if (metainfo_charset)
-		convert_to_utf8(line, endline - line, charset);
+		convert_to_utf8(line, charset.buf);
 
 	if (patchbreak(line)) {
 		fclose(cmitmsg);
@@ -756,142 +714,132 @@ static int handle_commit_msg(char *line, unsigned linesize)
 		return 1;
 	}
 
-	fputs(line, cmitmsg);
+	fputs(line->buf, cmitmsg);
 	return 0;
 }
 
-static int handle_patch(char *line, int len)
+static void handle_patch(const struct strbuf *line)
 {
-	fwrite(line, 1, len, patchfile);
+	fwrite(line->buf, 1, line->len, patchfile);
 	patch_lines++;
-	return 0;
 }
 
-static int handle_filter(char *line, unsigned linesize, int linelen)
+static void handle_filter(struct strbuf *line)
 {
 	static int filter = 0;
 
-	/* filter tells us which part we left off on
-	 * a non-zero return indicates we hit a filter point
-	 */
+	/* filter tells us which part we left off on */
 	switch (filter) {
 	case 0:
-		if (!handle_commit_msg(line, linesize))
+		if (!handle_commit_msg(line))
 			break;
 		filter++;
 	case 1:
-		if (!handle_patch(line, linelen))
-			break;
-		filter++;
-	default:
-		return 1;
+		handle_patch(line);
+		break;
 	}
-
-	return 0;
 }
 
 static void handle_body(void)
 {
-	int rc = 0;
-	static char newline[2000];
-	static char *np = newline;
-	int len = strlen(line);
+	int len = 0;
+	struct strbuf prev = STRBUF_INIT;
 
 	/* Skip up to the first boundary */
-	if (content_top->boundary) {
+	if (*content_top) {
 		if (!find_boundary())
-			return;
+			goto handle_body_out;
 	}
 
 	do {
+		strbuf_setlen(&line, line.len + len);
+
 		/* process any boundary lines */
-		if (content_top->boundary && is_multipart_boundary(line)) {
+		if (*content_top && is_multipart_boundary(&line)) {
 			/* flush any leftover */
-			if (np != newline)
-				handle_filter(newline, sizeof(newline),
-					      np - newline);
+			if (line.len)
+				handle_filter(&line);
+
 			if (!handle_boundary())
-				return;
-			len = strlen(line);
+				goto handle_body_out;
 		}
 
 		/* Unwrap transfer encoding */
-		len = decode_transfer_encoding(line, sizeof(line), len);
-		if (len < 0) {
-			error("Malformed input line");
-			return;
-		}
+		decode_transfer_encoding(&line);
 
 		switch (transfer_encoding) {
 		case TE_BASE64:
 		case TE_QP:
 		{
-			char *op = line;
+			struct strbuf **lines, **it, *sb;
+
+			/* Prepend any previous partial lines */
+			strbuf_insert(&line, 0, prev.buf, prev.len);
+			strbuf_reset(&prev);
 
 			/* binary data most likely doesn't have newlines */
 			if (message_type != TYPE_TEXT) {
-				rc = handle_filter(line, sizeof(line), len);
+				handle_filter(&line);
 				break;
 			}
-
 			/*
 			 * This is a decoded line that may contain
 			 * multiple new lines.  Pass only one chunk
 			 * at a time to handle_filter()
 			 */
-			do {
-				while (op < line + len && *op != '\n')
-					*np++ = *op++;
-				*np = *op;
-				if (*np != 0) {
-					/* should be sitting on a new line */
-					*(++np) = 0;
-					op++;
-					rc = handle_filter(newline, sizeof(newline), np - newline);
-					np = newline;
-				}
-			} while (op < line + len);
+			lines = strbuf_split(&line, '\n');
+			for (it = lines; (sb = *it); it++) {
+				if (*(it + 1) == NULL) /* The last line */
+					if (sb->buf[sb->len - 1] != '\n') {
+						/* Partial line, save it for later. */
+						strbuf_addbuf(&prev, sb);
+						break;
+					}
+				handle_filter(sb);
+			}
 			/*
-			 * The partial chunk is saved in newline and will be
+			 * The partial chunk is saved in "prev" and will be
 			 * appended by the next iteration of read_line_with_nul().
 			 */
+			strbuf_list_free(lines);
 			break;
 		}
 		default:
-			rc = handle_filter(line, sizeof(line), len);
+			handle_filter(&line);
 		}
-		if (rc)
-			/* nothing left to filter */
-			break;
-	} while ((len = read_line_with_nul(line, sizeof(line), fin)));
 
-	return;
+		strbuf_reset(&line);
+		if (strbuf_avail(&line) < 100)
+			strbuf_grow(&line, 100);
+	} while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin)));
+
+handle_body_out:
+	strbuf_release(&prev);
 }
 
-static void output_header_lines(FILE *fout, const char *hdr, char *data)
+static void output_header_lines(FILE *fout, const char *hdr, const struct strbuf *data)
 {
+	const char *sp = data->buf;
 	while (1) {
-		char *ep = strchr(data, '\n');
+		char *ep = strchr(sp, '\n');
 		int len;
 		if (!ep)
-			len = strlen(data);
+			len = strlen(sp);
 		else
-			len = ep - data;
-		fprintf(fout, "%s: %.*s\n", hdr, len, data);
+			len = ep - sp;
+		fprintf(fout, "%s: %.*s\n", hdr, len, sp);
 		if (!ep)
 			break;
-		data = ep + 1;
+		sp = ep + 1;
 	}
 }
 
 static void handle_info(void)
 {
-	char *sub;
-	char *hdr;
+	struct strbuf *hdr;
 	int i;
 
 	for (i = 0; header[i]; i++) {
-
 		/* only print inbody headers if we output a patch file */
 		if (patch_lines && s_hdr_data[i])
 			hdr = s_hdr_data[i];
@@ -901,20 +849,18 @@ static void handle_info(void)
 			continue;
 
 		if (!memcmp(header[i], "Subject", 7)) {
-			if (keep_subject)
-				sub = hdr;
-			else {
-				sub = cleanup_subject(hdr);
-				cleanup_space(sub);
+			if (!keep_subject) {
+				cleanup_subject(hdr);
+				cleanup_space(hdr);
 			}
-			output_header_lines(fout, "Subject", sub);
+			output_header_lines(fout, "Subject", hdr);
 		} else if (!memcmp(header[i], "From", 4)) {
 			handle_from(hdr);
-			fprintf(fout, "Author: %s\n", name);
-			fprintf(fout, "Email: %s\n", email);
+			fprintf(fout, "Author: %s\n", name.buf);
+			fprintf(fout, "Email: %s\n", email.buf);
 		} else {
 			cleanup_space(hdr);
-			fprintf(fout, "%s: %s\n", header[i], hdr);
+			fprintf(fout, "%s: %s\n", header[i], hdr->buf);
 		}
 	}
 	fprintf(fout, "\n");
@@ -941,8 +887,8 @@ static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
 		return -1;
 	}
 
-	p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(char *));
-	s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(char *));
+	p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*p_hdr_data));
+	s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*s_hdr_data));
 
 	do {
 		peek = fgetc(in);
@@ -950,8 +896,8 @@ static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
 	ungetc(peek, in);
 
 	/* process the email header */
-	while (read_one_header_line(line, sizeof(line), fin))
-		check_header(line, sizeof(line), p_hdr_data, 1);
+	while (read_one_header_line(&line, fin))
+		check_header(&line, p_hdr_data, 1);
 
 	handle_body();
 	handle_info();
-- 
1.5.4.5

^ permalink raw reply related

* Re: [RFH] Finding all commits that touch the same files as a specific commit
From: Johannes Schindelin @ 2008-07-13 18:30 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Junio C Hamano, Git Mailinglist
In-Reply-To: <bd6139dc0807130743y178a865amebcf130a68283854@mail.gmail.com>

Hi,

On Sun, 13 Jul 2008, Sverre Rabbelier wrote:

> On Sun, Jul 13, 2008 at 3:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> <explanation of the git log traversal machinery snipped>
> 
> > In order to follow renames reliably in a merge heavy history, you need to
> > keep track of the pathname the file you are interested in appears as _in
> > each commit_.  As you traverse down the history, you pass down the
> > pathname to the parent you visit, so while you are traversing from 'x' to
> > earlier 'x', you will keep following "git-gui/git-gui.sh", while you
> > traverse down to 'o', you will inspect "git-gui.sh".
> >
> > The data structure the revision traversal machinery uses does not support
> > this "path-per-commit" natively.
> 
> Would it be possible to go for a slightly less complicated approach
> and instead of passing replacing the tracked file, append it?

Maybe I miss something, but do you not have to keep track of the file 
names, in order to keep track of the proper statistics?

If that is the case, appending does not cut it.

Ciao,
Dscho

^ permalink raw reply

* Re: tests for git-p4
From: Matt McClure @ 2008-07-13 18:40 UTC (permalink / raw)
  To: Simon Hausmann; +Cc: git
In-Reply-To: <200807131426.09688.simon@lst.de>

On Sun, Jul 13, 2008 at 8:26 AM, Simon Hausmann <simon@lst.de> wrote:
> On Saturday 12 July 2008 14:24:10 Matt McClure wrote:
> There are currently no automated tests for git-p4. I would certainly be
> interested, but I forsee difficulties testing the interaction with p4 in a
> general way independent from a specific server setup.

I can solve that by bootstrapping a p4d server for the tests that need
to exercise p4 directly.  At higher levels of abstraction, if they
evolve, I could replace p4 with a test double that doesn't need a
Perforce server.

Matt

^ permalink raw reply

* Re: [PATCH/Test] Build in merge is broken
From: Miklos Vajna @ 2008-07-13 18:43 UTC (permalink / raw)
  To: Sverre Hvammen Johansen; +Cc: git, Junio C Hamano
In-Reply-To: <20080713174659.GE10347@genesis.frugalware.org>

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

On Sun, Jul 13, 2008 at 07:46:59PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> So far what I see is that the input for the reduce_heads() function is
> (c1, c0, c2, c0, c1). The expected output would be (c1, c2), but the
> actual output is c2. So I suspect the bug is not in builtin-merge.c
> itself but in reduce_heads().

This fixes the problem for me. Junio, does the fix looks correct to you
as well?

Thanks.

diff --git a/commit.c b/commit.c
index d20b14e..03e73f3 100644
--- a/commit.c
+++ b/commit.c
@@ -747,7 +747,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
 
 		num_other = 0;
 		for (q = heads; q; q = q->next) {
-			if (p == q)
+			if (p->item == q->item)
 				continue;
 			other[num_other++] = q->item;
 		}

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

^ permalink raw reply related

* Re: [PATCH 0/4] Proposed patches
From: Lukas Sandström @ 2008-07-13 18:57 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0807130442k36a10747q7bcbd996cdc774dd@mail.gmail.com>

Catalin Marinas wrote:
> 2008/7/13 Catalin Marinas <catalin.marinas@gmail.com>:
>> A new post of the proposed patches after implementing Karl's comments.
> 
> BTW, these are StGIT patches, I forgot to add the prefix in the subject line.
> 
If you run "git config format.subjectprefix 'StGIT PATCH'" in your repo,
git format-patch will add the prefix automatically for you.

StGIT might have a similar option.

/Lukas

^ permalink raw reply

* Re: [PATCH/Test] Build in merge is broken
From: Junio C Hamano @ 2008-07-13 19:11 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Sverre Hvammen Johansen, git
In-Reply-To: <20080713184300.GF10347@genesis.frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> On Sun, Jul 13, 2008 at 07:46:59PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> So far what I see is that the input for the reduce_heads() function is
>> (c1, c0, c2, c0, c1). The expected output would be (c1, c2), but the
>> actual output is c2. So I suspect the bug is not in builtin-merge.c
>> itself but in reduce_heads().
>
> This fixes the problem for me. Junio, does the fix looks correct to you
> as well?

You are correct, the "item"s are the highlander (i.e. "there can be only
one") objects but commit-list elements that hold pointers to them are not,
so we need to dereference and compare.

Thanks.

^ permalink raw reply

* Re: [PATCH] Improve sed portability
From: Jakub Narebski @ 2008-07-13 20:00 UTC (permalink / raw)
  To: Chris Ridd
In-Reply-To: <484FEF71.2030909@isode.com>

Chris Ridd <chris.ridd@isode.com> writes:

> Of course, using a plain old:
> 
>      echo "$1"
> 
> should work well too. Why is printf being used here and not echo,
> anyway?

Uh, because 'echo -n' is not portable enough, and for some reason it
was though that there shouldn't be final newline?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: found some nice git scripts
From: Petr Baudis @ 2008-07-13 20:12 UTC (permalink / raw)
  To: Raimund Bauer; +Cc: git, gitorious
In-Reply-To: <1214599789.6576.1.camel@doriath>

On Fri, Jun 27, 2008 at 10:49:49PM +0200, Raimund Bauer wrote:
> Maybe somebody else find them useful too:
> http://git-wt-commit.rubyforge.org/

  Interesting. I wonder if the William Morgan knows about git
show-branch? It provides very similar functionality, but soemwhat
different kind of output; extending it with git-wtf-like functionality
(that is, per-branch "merge status summary", and "flat" view of commits)
might be quite easy and practical for others at the same time.

  About, git-publish-branch, again, maybe it would make a great deal of
sense to teach git-push a --track flag?

  I understand if the author has not much desire to reimplement this in
C, on the other hand everyone would benefit from builtin support; maybe
someone else gets inspired.

-- 
				Petr "Pasky" Baudis
GNU, n. An animal of South Africa, which in its domesticated state
resembles a horse, a buffalo and a stag. In its wild condition it is
something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce

^ permalink raw reply

* Re: [PATCH] Added --export option to git-send-email.
From: Junio C Hamano @ 2008-07-13 20:17 UTC (permalink / raw)
  To: Eduard - Gabriel Munteanu; +Cc: ryan, git
In-Reply-To: <20080710170735.4c5b237a@linux360.ro>

Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> writes:

> diff --git a/git-send-email.perl b/git-send-email.perl
> index 3564419..53539cf 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -741,6 +747,10 @@ X-Mailer: git-send-email $gitversion
>  
>  	if ($dry_run) {
>  		# We don't want to send the email.
> +	} elsif (defined $export_file) {
> +		my $mbox_from = "From $raw_from $date";
> +		$message =~ s/^(>{0,})From/>$1From/m;
> +		print $out "$mbox_from\n$header\n$message";

Sorry, but I am scratching my head here.  Shouldn't this "bare From
munging" be like this instead?

		$message =~ s/^From />From /m;

^ permalink raw reply

* [PATCH] help.c: Add support for htmldir relative to git_exec_path()
From: Johannes Sixt @ 2008-07-13 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin, Johannes Sixt
In-Reply-To: <1215981083-10815-1-git-send-email-johannes.sixt@telecom.at>

From: Steffen Prohaska <prohaska@zib.de>

If htmldir (in the Makefile) is a relative path, this path will now be
interpreted relative to git_exec_path.  This can be used to create an
installation that can be moved to a different directory without
re-compiling.  The Windows installer (msysgit) is an example for such
a setup.

Note that the Makefile maps htmldir to the define GIT_HTML_PATH.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 help.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/help.c b/help.c
index ca9632b..0f055bf 100644
--- a/help.c
+++ b/help.c
@@ -633,13 +633,15 @@ static void show_info_page(const char *git_cmd)
 static void get_html_page_path(struct strbuf *page_path, const char *page)
 {
 	struct stat st;
+	const char *html_path = system_path(GIT_HTML_PATH);
 
 	/* Check that we have a git documentation directory. */
-	if (stat(GIT_HTML_PATH "/git.html", &st) || !S_ISREG(st.st_mode))
-		die("'%s': not a documentation directory.", GIT_HTML_PATH);
+	if (stat(mkpath("%s/git.html", html_path), &st)
+	    || !S_ISREG(st.st_mode))
+		die("'%s': not a documentation directory.", html_path);
 
 	strbuf_init(page_path, 0);
-	strbuf_addf(page_path, GIT_HTML_PATH "/%s.html", page);
+	strbuf_addf(page_path, "%s/%s.html", html_path, page);
 }
 
 static void show_html_page(const char *git_cmd)
-- 
1.5.6.2.300.ga3a9

^ permalink raw reply related

* [PATCH] Move code interpreting path relative to exec-dir to new function system_path()
From: Johannes Sixt @ 2008-07-13 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin, Johannes Sixt
In-Reply-To: <7v1w1yupn4.fsf@gitster.siamese.dyndns.org>

From: Steffen Prohaska <prohaska@zib.de>

Expanding system paths relative to git_exec_path can be used for
creating an installation that can be moved to a different directory
without re-compiling.  We use this approach for template_dir and the
system wide gitconfig.  The Windows installer (msysgit) is an example
for such a setup.

This commit moves common code to a new function system_path().  System
paths that are to be interpreted relative to git_exec_path are passed to
system_path() and the return value is used instead of the original path.
system_path() prefixes a relative path with git_exec_path and leaves
absolute paths unmodified.  For example, we now write

    template_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);

[j6t: moved from path.c to exec_cmd.c]

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 builtin-init-db.c |   14 ++------------
 config.c          |   11 ++---------
 exec_cmd.c        |   10 ++++++++++
 exec_cmd.h        |    2 +-
 4 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/builtin-init-db.c b/builtin-init-db.c
index e23b843..5ba213a 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -115,18 +115,8 @@ static void copy_templates(const char *template_dir)
 
 	if (!template_dir)
 		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
-	if (!template_dir) {
-		/*
-		 * if the hard-coded template is relative, it is
-		 * interpreted relative to the exec_dir
-		 */
-		template_dir = DEFAULT_GIT_TEMPLATE_DIR;
-		if (!is_absolute_path(template_dir)) {
-			struct strbuf d = STRBUF_INIT;
-			strbuf_addf(&d, "%s/%s", git_exec_path(), template_dir);
-			template_dir = strbuf_detach(&d, NULL);
-		}
-	}
+	if (!template_dir)
+		template_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);
 	strcpy(template_path, template_dir);
 	template_len = strlen(template_path);
 	if (template_path[template_len-1] != '/') {
diff --git a/config.c b/config.c
index 2862cc4..1e066c7 100644
--- a/config.c
+++ b/config.c
@@ -581,15 +581,8 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 const char *git_etc_gitconfig(void)
 {
 	static const char *system_wide;
-	if (!system_wide) {
-		system_wide = ETC_GITCONFIG;
-		if (!is_absolute_path(system_wide)) {
-			/* interpret path relative to exec-dir */
-			struct strbuf d = STRBUF_INIT;
-			strbuf_addf(&d, "%s/%s", git_exec_path(), system_wide);
-			system_wide = strbuf_detach(&d, NULL);
-		}
-	}
+	if (!system_wide)
+		system_wide = system_path(ETC_GITCONFIG);
 	return system_wide;
 }
 
diff --git a/exec_cmd.c b/exec_cmd.c
index da04efe..8899e31 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -40,6 +40,16 @@ static const char *builtin_exec_path(void)
 #endif
 }
 
+const char *system_path(const char *path)
+{
+	if (!is_absolute_path(path)) {
+		struct strbuf d = STRBUF_INIT;
+		strbuf_addf(&d, "%s/%s", git_exec_path(), path);
+		path = strbuf_detach(&d, NULL);
+	}
+	return path;
+}
+
 void git_set_argv_exec_path(const char *exec_path)
 {
 	argv_exec_path = exec_path;
diff --git a/exec_cmd.h b/exec_cmd.h
index a892355..7eb94e5 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -6,6 +6,6 @@ extern const char* git_exec_path(void);
 extern void setup_path(const char *);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
-
+extern const char *system_path(const char *path);
 
 #endif /* GIT_EXEC_CMD_H */
-- 
1.5.6.2.300.ga3a9

^ permalink raw reply related

* [PATCH] help (Windows): Display HTML in default browser using Windows' shell API
From: Johannes Sixt @ 2008-07-13 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin, Johannes Sixt
In-Reply-To: <1215981083-10815-2-git-send-email-johannes.sixt@telecom.at>

From: Steffen Prohaska <prohaska@zib.de>

The system's default browser for displaying HTML help pages is now used
directly on Windows, instead of launching git-web--browser, which
requires a Unix shell.  Avoiding MSYS' bash when possible is good
because it avoids potential path translation issues.  In this case it is
not too hard to avoid launching a shell, so let's avoid it.

The Windows-specific code is implemented in compat/mingw.c to avoid
platform-specific code in the main code base.  On Windows, open_html is
provided as a define.  If open_html is not defined, git-web--browse is
used.  This approach avoids platform-specific ifdefs by using
per-function ifdefs.  The "ifndef open_html" together with the
introductory comment should sufficiently warn developers, so that they
hopefully will not break this mechanism.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 compat/mingw.c |   21 +++++++++++++++++++++
 compat/mingw.h |    3 +++
 help.c         |   14 +++++++++++++-
 3 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 3a05fe7..0ca73f7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1017,3 +1017,24 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler)
 	timer_fn = handler;
 	return old;
 }
+
+static const char *make_backslash_path(const char *path) {
+	static char buf[PATH_MAX + 1];
+	char *c;
+
+	if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
+		die ("Too long path: %.*s", 60, path);
+
+	for (c = buf; *c; c++) {
+		if (*c == '/')
+			*c = '\\';
+	}
+	return buf;
+}
+
+void mingw_open_html(const char *unixpath)
+{
+	const char *htmlpath = make_backslash_path(unixpath);
+	printf("Launching default browser to display HTML ...\n");
+	ShellExecute(NULL, "open", htmlpath, NULL, "\\", 0);
+}
diff --git a/compat/mingw.h b/compat/mingw.h
index 6bc049a..5a3bcee 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -202,6 +202,9 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler);
 #define PATH_SEP ';'
 #define PRIuMAX "I64u"
 
+void mingw_open_html(const char *path);
+#define open_html mingw_open_html
+
 /*
  * helpers
  */
diff --git a/help.c b/help.c
index 0f055bf..52d39b8 100644
--- a/help.c
+++ b/help.c
@@ -644,6 +644,18 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
 	strbuf_addf(page_path, "%s/%s.html", html_path, page);
 }
 
+/*
+ * If open_html is not defined in a platform-specific way (see for
+ * example compat/mingw.h), we use the script web--browse to display
+ * HTML.
+ */
+#ifndef open_html
+void open_html(const char *path)
+{
+	execl_git_cmd("web--browse", "-c", "help.browser", path, NULL);
+}
+#endif
+
 static void show_html_page(const char *git_cmd)
 {
 	const char *page = cmd_to_page(git_cmd);
@@ -651,7 +663,7 @@ static void show_html_page(const char *git_cmd)
 
 	get_html_page_path(&page_path, page);
 
-	execl_git_cmd("web--browse", "-c", "help.browser", page_path.buf, NULL);
+	open_html(page_path.buf);
 }
 
 void help_unknown_cmd(const char *cmd)
-- 
1.5.6.2.300.ga3a9

^ permalink raw reply related

* [PATCH] Fix relative built-in paths to be relative to the command invocation
From: Johannes Sixt @ 2008-07-13 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin, Johannes Sixt
In-Reply-To: <1215981083-10815-3-git-send-email-johannes.sixt@telecom.at>

$(gitexecdir) (as defined in the Makefile) has gained another path
component, but the relative paths in the MINGW section of the Makefile,
which are interpreted relative to it, do not account for it.

Instead of adding another ../ in front of the path, we change the code that
constructs the absolute paths to do it relative to the command's directory,
which is essentially $(bindir). We do it this way because we will also
allow a relative $(gitexecdir) later.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile       |    2 +-
 exec_cmd.c     |   14 ++++++++++----
 exec_cmd.h     |    3 ++-
 git.c          |    5 ++---
 receive-pack.c |    2 +-
 shell.c        |    4 ++--
 upload-pack.c  |    2 +-
 7 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 4796565..2bdb9bf 100644
--- a/Makefile
+++ b/Makefile
@@ -1301,7 +1301,7 @@ remove-dashes:
 ### Installation rules
 
 ifeq ($(firstword $(subst /, ,$(template_dir))),..)
-template_instdir = $(gitexecdir)/$(template_dir)
+template_instdir = $(shell cd '$(bindir_SQ)/$(template_dir_SQ)' && pwd)
 else
 template_instdir = $(template_dir)
 endif
diff --git a/exec_cmd.c b/exec_cmd.c
index 8899e31..45f92eb 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -5,6 +5,7 @@
 
 extern char **environ;
 static const char *argv_exec_path;
+static const char *argv0_path;
 
 static const char *builtin_exec_path(void)
 {
@@ -42,14 +43,19 @@ static const char *builtin_exec_path(void)
 
 const char *system_path(const char *path)
 {
-	if (!is_absolute_path(path)) {
+	if (!is_absolute_path(path) && argv0_path) {
 		struct strbuf d = STRBUF_INIT;
-		strbuf_addf(&d, "%s/%s", git_exec_path(), path);
+		strbuf_addf(&d, "%s/%s", argv0_path, path);
 		path = strbuf_detach(&d, NULL);
 	}
 	return path;
 }
 
+void git_set_argv0_path(const char *path)
+{
+	argv0_path = path;
+}
+
 void git_set_argv_exec_path(const char *exec_path)
 {
 	argv_exec_path = exec_path;
@@ -84,7 +90,7 @@ static void add_path(struct strbuf *out, const char *path)
 	}
 }
 
-void setup_path(const char *cmd_path)
+void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path;
@@ -94,7 +100,7 @@ void setup_path(const char *cmd_path)
 	add_path(&new_path, argv_exec_path);
 	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
 	add_path(&new_path, builtin_exec_path());
-	add_path(&new_path, cmd_path);
+	add_path(&new_path, argv0_path);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/exec_cmd.h b/exec_cmd.h
index 7eb94e5..0c46cd5 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -2,8 +2,9 @@
 #define GIT_EXEC_CMD_H
 
 extern void git_set_argv_exec_path(const char *exec_path);
+extern void git_set_argv0_path(const char *path);
 extern const char* git_exec_path(void);
-extern void setup_path(const char *);
+extern void setup_path(void);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 extern const char *system_path(const char *path);
diff --git a/git.c b/git.c
index 7075533..b90c358 100644
--- a/git.c
+++ b/git.c
@@ -470,7 +470,6 @@ int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
 	char *slash = (char *)cmd + strlen(cmd);
-	const char *cmd_path = NULL;
 	int done_alias = 0;
 
 	/*
@@ -483,7 +482,7 @@ int main(int argc, const char **argv)
 	while (cmd <= slash && !is_dir_sep(*slash));
 	if (cmd <= slash) {
 		*slash++ = 0;
-		cmd_path = cmd;
+		git_set_argv0_path(cmd);
 		cmd = slash;
 	}
 
@@ -527,7 +526,7 @@ int main(int argc, const char **argv)
 	 * environment, and the $(gitexecdir) from the Makefile at build
 	 * time.
 	 */
-	setup_path(cmd_path);
+	setup_path();
 
 	while (1) {
 		/* See if it's an internal command */
diff --git a/receive-pack.c b/receive-pack.c
index fa653b4..d44c19e 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -482,7 +482,7 @@ int main(int argc, char **argv)
 	if (!dir)
 		usage(receive_pack_usage);
 
-	setup_path(NULL);
+	setup_path();
 
 	if (!enter_repo(dir, 0))
 		die("'%s': unable to chdir or not a git archive", dir);
diff --git a/shell.c b/shell.c
index 91ca7de..6a48de0 100644
--- a/shell.c
+++ b/shell.c
@@ -15,7 +15,7 @@ static int do_generic_cmd(const char *me, char *arg)
 {
 	const char *my_argv[4];
 
-	setup_path(NULL);
+	setup_path();
 	if (!arg || !(arg = sq_dequote(arg)))
 		die("bad argument");
 	if (prefixcmp(me, "git-"))
@@ -37,7 +37,7 @@ static int do_cvs_cmd(const char *me, char *arg)
 	if (!arg || strcmp(arg, "server"))
 		die("git-cvsserver only handles server: %s", arg);
 
-	setup_path(NULL);
+	setup_path();
 	return execv_git_cmd(cvsserver_argv);
 }
 
diff --git a/upload-pack.c b/upload-pack.c
index 9f82941..c911e70 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -638,7 +638,7 @@ int main(int argc, char **argv)
 	if (i != argc-1)
 		usage(upload_pack_usage);
 
-	setup_path(NULL);
+	setup_path();
 
 	dir = argv[i];
 
-- 
1.5.6.2.300.ga3a9

^ permalink raw reply related

* [PATCH] Allow add_path() to add non-existent directories to the path
From: Johannes Sixt @ 2008-07-13 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin, Johannes Sixt
In-Reply-To: <1215981083-10815-5-git-send-email-johannes.sixt@telecom.at>

This function had used make_absolute_path(); but this function dies if
the directory that contains the entry whose relative path was supplied in
the argument does not exist. This is a problem if the argument is, for
example, "../libexec/git-core", and that "../libexec" does not exist.

Since the resolution of symbolic links is not required for elements in
PATH, we can fall back to using make_nonrelative_path(), which simply
prepends $PWD to the path.

We have to move make_nonrelative_path() alongside make_absolute_path() in
abspath.c so that git-shell can be linked. See 5b8e6f85f.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 abspath.c  |   36 ++++++++++++++++++++++++++++++++++++
 exec_cmd.c |    2 +-
 path.c     |   36 ------------------------------------
 3 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/abspath.c b/abspath.c
index 4f95a95..99ee1af 100644
--- a/abspath.c
+++ b/abspath.c
@@ -66,3 +66,39 @@ const char *make_absolute_path(const char *path)
 
 	return buf;
 }
+
+static const char *get_pwd_cwd(void)
+{
+	static char cwd[PATH_MAX + 1];
+	char *pwd;
+	struct stat cwd_stat, pwd_stat;
+	if (getcwd(cwd, PATH_MAX) == NULL)
+		return NULL;
+	pwd = getenv("PWD");
+	if (pwd && strcmp(pwd, cwd)) {
+		stat(cwd, &cwd_stat);
+		if (!stat(pwd, &pwd_stat) &&
+		    pwd_stat.st_dev == cwd_stat.st_dev &&
+		    pwd_stat.st_ino == cwd_stat.st_ino) {
+			strlcpy(cwd, pwd, PATH_MAX);
+		}
+	}
+	return cwd;
+}
+
+const char *make_nonrelative_path(const char *path)
+{
+	static char buf[PATH_MAX + 1];
+
+	if (is_absolute_path(path)) {
+		if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
+			die ("Too long path: %.*s", 60, path);
+	} else {
+		const char *cwd = get_pwd_cwd();
+		if (!cwd)
+			die("Cannot determine the current working directory");
+		if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path) >= PATH_MAX)
+			die ("Too long path: %.*s", 60, path);
+	}
+	return buf;
+}
diff --git a/exec_cmd.c b/exec_cmd.c
index c236034..0ed768d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -50,7 +50,7 @@ static void add_path(struct strbuf *out, const char *path)
 		if (is_absolute_path(path))
 			strbuf_addstr(out, path);
 		else
-			strbuf_addstr(out, make_absolute_path(path));
+			strbuf_addstr(out, make_nonrelative_path(path));
 
 		strbuf_addch(out, PATH_SEP);
 	}
diff --git a/path.c b/path.c
index 5983255..16c1d01 100644
--- a/path.c
+++ b/path.c
@@ -291,42 +291,6 @@ int adjust_shared_perm(const char *path)
 	return 0;
 }
 
-static const char *get_pwd_cwd(void)
-{
-	static char cwd[PATH_MAX + 1];
-	char *pwd;
-	struct stat cwd_stat, pwd_stat;
-	if (getcwd(cwd, PATH_MAX) == NULL)
-		return NULL;
-	pwd = getenv("PWD");
-	if (pwd && strcmp(pwd, cwd)) {
-		stat(cwd, &cwd_stat);
-		if (!stat(pwd, &pwd_stat) &&
-		    pwd_stat.st_dev == cwd_stat.st_dev &&
-		    pwd_stat.st_ino == cwd_stat.st_ino) {
-			strlcpy(cwd, pwd, PATH_MAX);
-		}
-	}
-	return cwd;
-}
-
-const char *make_nonrelative_path(const char *path)
-{
-	static char buf[PATH_MAX + 1];
-
-	if (is_absolute_path(path)) {
-		if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
-			die ("Too long path: %.*s", 60, path);
-	} else {
-		const char *cwd = get_pwd_cwd();
-		if (!cwd)
-			die("Cannot determine the current working directory");
-		if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path) >= PATH_MAX)
-			die ("Too long path: %.*s", 60, path);
-	}
-	return buf;
-}
-
 const char *make_relative_path(const char *abs, const char *base)
 {
 	static char buf[PATH_MAX + 1];
-- 
1.5.6.2.300.ga3a9

^ permalink raw reply related

* [PATCH] Allow the built-in exec path to be relative to the command invocation path
From: Johannes Sixt @ 2008-07-13 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin, Johannes Sixt
In-Reply-To: <1215981083-10815-4-git-send-email-johannes.sixt@telecom.at>

If GIT_EXEC_PATH (the macro that is defined in the Makefile) is relative,
it is interpreted relative to the command's invocation path, which usually
is $(bindir).

The Makefile rules were written with the assumption that $(gitexecdir) is
an absolute path. We introduce a separate variable that names the
(absolute) installation directory.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile   |   23 +++++++++++++++--------
 exec_cmd.c |   38 ++------------------------------------
 2 files changed, 17 insertions(+), 44 deletions(-)

diff --git a/Makefile b/Makefile
index 2bdb9bf..3593e6f 100644
--- a/Makefile
+++ b/Makefile
@@ -1307,10 +1307,17 @@ template_instdir = $(template_dir)
 endif
 export template_instdir
 
+ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
+gitexec_instdir = $(shell cd '$(bindir_SQ)/$(gitexecdir_SQ)' && pwd)
+else
+gitexec_instdir = $(gitexecdir)
+endif
+gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
+
 install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
@@ -1318,18 +1325,18 @@ ifndef NO_TCLTK
 	$(MAKE) -C gitk-git install
 	$(MAKE) -C git-gui install
 endif
-	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
+	if test 'z$(bindir_SQ)' != 'z$(gitexec_instdir_SQ)'; \
 	then \
 		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
-			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' || \
+			'$(DESTDIR_SQ)$(gitexec_instdir_SQ)/git$X' || \
 		cp '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
-			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
+			'$(DESTDIR_SQ)$(gitexec_instdir_SQ)/git$X'; \
 	fi
-	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
+	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' ;)
 ifneq (,$X)
-	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
 endif
-	./check_bindir 'z$(bindir_SQ)' 'z$(gitexecdir_SQ)' '$(DESTDIR_SQ)$(bindir_SQ)/git-shell$X'
+	./check_bindir 'z$(bindir_SQ)' 'z$(gitexec_instdir_SQ)' '$(DESTDIR_SQ)$(bindir_SQ)/git-shell$X'
 
 install-doc:
 	$(MAKE) -C Documentation install
diff --git a/exec_cmd.c b/exec_cmd.c
index 45f92eb..c236034 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -7,40 +7,6 @@ extern char **environ;
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-static const char *builtin_exec_path(void)
-{
-#ifndef __MINGW32__
-	return GIT_EXEC_PATH;
-#else
-	int len;
-	char *p, *q, *sl;
-	static char *ep;
-	if (ep)
-		return ep;
-
-	len = strlen(_pgmptr);
-	if (len < 2)
-		return ep = ".";
-
-	p = ep = xmalloc(len+1);
-	q = _pgmptr;
-	sl = NULL;
-	/* copy program name, turn '\\' into '/', skip last part */
-	while ((*p = *q)) {
-		if (*q == '\\' || *q == '/') {
-			*p = '/';
-			sl = p;
-		}
-		p++, q++;
-	}
-	if (sl)
-		*sl = '\0';
-	else
-		ep[0] = '.', ep[1] = '\0';
-	return ep;
-#endif
-}
-
 const char *system_path(const char *path)
 {
 	if (!is_absolute_path(path) && argv0_path) {
@@ -75,7 +41,7 @@ const char *git_exec_path(void)
 		return env;
 	}
 
-	return builtin_exec_path();
+	return system_path(GIT_EXEC_PATH);
 }
 
 static void add_path(struct strbuf *out, const char *path)
@@ -99,7 +65,7 @@ void setup_path(void)
 
 	add_path(&new_path, argv_exec_path);
 	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
-	add_path(&new_path, builtin_exec_path());
+	add_path(&new_path, system_path(GIT_EXEC_PATH));
 	add_path(&new_path, argv0_path);
 
 	if (old_path)
-- 
1.5.6.2.300.ga3a9

^ permalink raw reply related

* Re: [PATCH] gitk - work around stderr redirection on Cygwin
From: Mark Levedahl @ 2008-07-13 20:37 UTC (permalink / raw)
  To: Mark Levedahl, paulus, Junio C Hamano; +Cc: git
In-Reply-To: <1213462668-424-1-git-send-email-mlevedahl@gmail.com>

Mark Levedahl wrote:
> Cygwin is *still* shipping with antiquated Tcl 8.4.1, and will continue
> to do so for the indefinite future. This version does not understand
> the "2>@1" redirection syntax, so such redirection of stderr must be
> done using sh.
>   

Ping. This bug is in 1.5.6.x, and thus also in the current Cygwin git 
release: as a result, several gitk context menu items cause errors. (Let 
me know if I should resend the patch).

Mark

^ permalink raw reply

* Re: [PATCH] Fix relative built-in paths to be relative to the command invocation
From: Johannes Schindelin @ 2008-07-13 20:43 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Steffen Prohaska, git
In-Reply-To: <1215981083-10815-4-git-send-email-johannes.sixt@telecom.at>

Hi,

On Sun, 13 Jul 2008, Johannes Sixt wrote:

> diff --git a/Makefile b/Makefile
> index 4796565..2bdb9bf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1301,7 +1301,7 @@ remove-dashes:
>  ### Installation rules
>  
>  ifeq ($(firstword $(subst /, ,$(template_dir))),..)
> -template_instdir = $(gitexecdir)/$(template_dir)
> +template_instdir = $(shell cd '$(bindir_SQ)/$(template_dir_SQ)' && pwd)

What is this for?  Did the original line stop working?

> diff --git a/exec_cmd.c b/exec_cmd.c
> index 8899e31..45f92eb 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -5,6 +5,7 @@
>  
>  extern char **environ;
>  static const char *argv_exec_path;
> +static const char *argv0_path;
>  
>  static const char *builtin_exec_path(void)
>  {
> @@ -42,14 +43,19 @@ static const char *builtin_exec_path(void)
>  
>  const char *system_path(const char *path)
>  {
> -	if (!is_absolute_path(path)) {
> +	if (!is_absolute_path(path) && argv0_path) {
>  		struct strbuf d = STRBUF_INIT;
> -		strbuf_addf(&d, "%s/%s", git_exec_path(), path);
> +		strbuf_addf(&d, "%s/%s", argv0_path, path);
>  		path = strbuf_detach(&d, NULL);
>  	}
>  	return path;
>  }
>  
> +void git_set_argv0_path(const char *path)
> +{
> +	argv0_path = path;
> +}
> +
>  void git_set_argv_exec_path(const char *exec_path)
>  {
>  	argv_exec_path = exec_path;
> @@ -84,7 +90,7 @@ static void add_path(struct strbuf *out, const char *path)
>  	}
>  }
>  
> -void setup_path(const char *cmd_path)
> +void setup_path(void)

It seems to me that this patch would not do anything different, but with 
less code change, if setup_path() would set argv0_path, and not a new 
function was introduced.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Allow the built-in exec path to be relative to the command invocation path
From: Johannes Schindelin @ 2008-07-13 20:45 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Steffen Prohaska, git
In-Reply-To: <1215981083-10815-5-git-send-email-johannes.sixt@telecom.at>

Hi,

On Sun, 13 Jul 2008, Johannes Sixt wrote:

> [a patch series, with 3 patches from Steffen]

Wow.  I like this demonstration how a nice patch series looks like.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 0/4] Proposed patches
From: Catalin Marinas @ 2008-07-13 21:10 UTC (permalink / raw)
  To: Lukas Sandström; +Cc: git
In-Reply-To: <487A5032.9090307@etek.chalmers.se>

2008/7/13 Lukas Sandström <lukass@etek.chalmers.se>:
> Catalin Marinas wrote:
>> 2008/7/13 Catalin Marinas <catalin.marinas@gmail.com>:
>>> A new post of the proposed patches after implementing Karl's comments.
>>
>> BTW, these are StGIT patches, I forgot to add the prefix in the subject line.
>>
> If you run "git config format.subjectprefix 'StGIT PATCH'" in your repo,
> git format-patch will add the prefix automatically for you.
>
> StGIT might have a similar option.

No, it doesn't, but it would be good to have one. The alternative in
StGIT is to modify the (local) template file.

Thanks,

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] git-bisect: use dash-less form on git bisect log
From: Junio C Hamano @ 2008-07-13 21:26 UTC (permalink / raw)
  To: Christian Couder; +Cc: Miklos Vajna, git
In-Reply-To: <200807130819.24206.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Your patch looks good. But there is also the problem that new bisect logs 
> will not be understood by old git. So I wonder if a patch to only read the 
> new log should be first commited to the "maint" branch.

I do not think it is worth it.  If we are making something long lived like
rerere cache backward incompatible, the approach would be sensible, but
bisect log is about a single session and once you reached the breakage you
would not reuse it.  Besides, the bisect log is designed to be executable
shell script so you can always copy it out, edit it if needed and feed
that to shell.

^ permalink raw reply

* Re: [PATCH] Added --export option to git-send-email.
From: Avery Pennarun @ 2008-07-13 21:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <7vskudr11m.fsf@gitster.siamese.dyndns.org>

On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> writes:
>  > +     } elsif (defined $export_file) {
>  > +             my $mbox_from = "From $raw_from $date";
>  > +             $message =~ s/^(>{0,})From/>$1From/m;
>  > +             print $out "$mbox_from\n$header\n$message";
>
> Sorry, but I am scratching my head here.  Shouldn't this "bare From
>  munging" be like this instead?
>
>                 $message =~ s/^From />From /m;

That wouldn't be a reversible operation.

Have fun,

Avery

^ permalink raw reply


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