[PATCH v1 1/3] dir.c: coding style fix

[PATCH v1 1/3] dir.c: coding style fix

[Karsten Blees]

From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=
 <pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Karsten Blees <blees@dcon.de>
---
 dir.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index e65888d..3068ca8 100644
--- a/dir.c
+++ b/dir.c
@@ -557,8 +557,7 @@ int add_excludes_from_file_to_list(const char *fname,
 			buf = xrealloc(buf, size+1);
 			buf[size++] = '\n';
 		}
-	}
-	else {
+	} else {
 		size = xsize_t(st.st_size);
 		if (size == 0) {
 			close(fd);
@@ -793,9 +792,11 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 
 	group = &dir->exclude_list_group[EXC_DIRS];
 
-	/* Pop the exclude lists from the EXCL_DIRS exclude_list_group
+	/*
+	 * Pop the exclude lists from the EXCL_DIRS exclude_list_group
 	 * which originate from directories not in the prefix of the
-	 * path being checked. */
+	 * path being checked.
+	 */
 	while ((stk = dir->exclude_stack) != NULL) {
 		if (stk->baselen <= baselen &&
 		    !strncmp(dir->basebuf, base, stk->baselen))
@@ -822,8 +823,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 		if (current < 0) {
 			cp = base;
 			current = 0;
-		}
-		else {
+		} else {
 			cp = strchr(base + current + 1, '/');
 			if (!cp)
 				die("oops in prep_exclude");
-- 
2.0.0.9646.g840d1f9.dirty

[PATCH v1 2/3] dir.h: move struct exclude declaration to top level

[Karsten Blees]

From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=
 <pclouds@gmail.com>

There is no actual nested struct here. Move it out for clarity.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Karsten Blees <blees@dcon.de>
---
 dir.h | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/dir.h b/dir.h
index 55e5345..02e3710 100644
--- a/dir.h
+++ b/dir.h
@@ -15,6 +15,27 @@ struct dir_entry {
 #define EXC_FLAG_MUSTBEDIR 8
 #define EXC_FLAG_NEGATIVE 16
 
+struct exclude {
+	/*
+	 * This allows callers of last_exclude_matching() etc.
+	 * to determine the origin of the matching pattern.
+	 */
+	struct exclude_list *el;
+
+	const char *pattern;
+	int patternlen;
+	int nowildcardlen;
+	const char *base;
+	int baselen;
+	int flags;
+
+	/*
+	 * Counting starts from 1 for line numbers in ignore files,
+	 * and from -1 decrementing for patterns from CLI args.
+	 */
+	int srcpos;
+};
+
 /*
  * Each excludes file will be parsed into a fresh exclude_list which
  * is appended to the relevant exclude_list_group (either EXC_DIRS or
@@ -32,26 +53,7 @@ struct exclude_list {
 	/* origin of list, e.g. path to filename, or descriptive string */
 	const char *src;
 
-	struct exclude {
-		/*
-		 * This allows callers of last_exclude_matching() etc.
-		 * to determine the origin of the matching pattern.
-		 */
-		struct exclude_list *el;
-
-		const char *pattern;
-		int patternlen;
-		int nowildcardlen;
-		const char *base;
-		int baselen;
-		int flags;
-
-		/*
-		 * Counting starts from 1 for line numbers in ignore files,
-		 * and from -1 decrementing for patterns from CLI args.
-		 */
-		int srcpos;
-	} **excludes;
+	struct exclude **excludes;
 };
 
 /*
-- 
2.0.0.9646.g840d1f9.dirty

[PATCH v1 3/3] prep_exclude: remove the artificial PATH_MAX limit

[Karsten Blees]

From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=
 <pclouds@gmail.com>

This fixes a segfault in git-status with long paths on Windows,
where PATH_MAX is only 260.

This also fixes the problem of silently ignoring .gitignore if the
full path exceeds PATH_MAX. Now add_excludes_from_file() will report
if it gets ENAMETOOLONG.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Karsten Blees <blees@dcon.de>
---
 dir.c | 47 ++++++++++++++++++++++++++++-------------------
 dir.h |  2 +-
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/dir.c b/dir.c
index 3068ca8..fcb6872 100644
--- a/dir.c
+++ b/dir.c
@@ -799,12 +799,12 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 	 */
 	while ((stk = dir->exclude_stack) != NULL) {
 		if (stk->baselen <= baselen &&
-		    !strncmp(dir->basebuf, base, stk->baselen))
+		    !strncmp(dir->basebuf.buf, base, stk->baselen))
 			break;
 		el = &group->el[dir->exclude_stack->exclude_ix];
 		dir->exclude_stack = stk->prev;
 		dir->exclude = NULL;
-		free((char *)el->src); /* see strdup() below */
+		free((char *)el->src); /* see strbuf_detach() below */
 		clear_exclude_list(el);
 		free(stk);
 		group->nr--;
@@ -814,8 +814,17 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 	if (dir->exclude)
 		return;
 
+	/*
+	 * Lazy initialization. All call sites currently just
+	 * memset(dir, 0, sizeof(*dir)) before use. Changing all of
+	 * them seems lots of work for little benefit.
+	 */
+	if (!dir->basebuf.buf)
+		strbuf_init(&dir->basebuf, PATH_MAX);
+
 	/* Read from the parent directories and push them down. */
 	current = stk ? stk->baselen : -1;
+	strbuf_setlen(&dir->basebuf, current < 0 ? 0 : current);
 	while (current < baselen) {
 		struct exclude_stack *stk = xcalloc(1, sizeof(*stk));
 		const char *cp;
@@ -833,48 +842,47 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 		stk->baselen = cp - base;
 		stk->exclude_ix = group->nr;
 		el = add_exclude_list(dir, EXC_DIRS, NULL);
-		memcpy(dir->basebuf + current, base + current,
-		       stk->baselen - current);
+		strbuf_add(&dir->basebuf, base + current, stk->baselen - current);
+		assert(stk->baselen == dir->basebuf.len);
 
 		/* Abort if the directory is excluded */
 		if (stk->baselen) {
 			int dt = DT_DIR;
-			dir->basebuf[stk->baselen - 1] = 0;
+			dir->basebuf.buf[stk->baselen - 1] = 0;
 			dir->exclude = last_exclude_matching_from_lists(dir,
-				dir->basebuf, stk->baselen - 1,
-				dir->basebuf + current, &dt);
-			dir->basebuf[stk->baselen - 1] = '/';
+				dir->basebuf.buf, stk->baselen - 1,
+				dir->basebuf.buf + current, &dt);
+			dir->basebuf.buf[stk->baselen - 1] = '/';
 			if (dir->exclude &&
 			    dir->exclude->flags & EXC_FLAG_NEGATIVE)
 				dir->exclude = NULL;
 			if (dir->exclude) {
-				dir->basebuf[stk->baselen] = 0;
 				dir->exclude_stack = stk;
 				return;
 			}
 		}
 
-		/* Try to read per-directory file unless path is too long */
-		if (dir->exclude_per_dir &&
-		    stk->baselen + strlen(dir->exclude_per_dir) < PATH_MAX) {
-			strcpy(dir->basebuf + stk->baselen,
-					dir->exclude_per_dir);
+		/* Try to read per-directory file */
+		if (dir->exclude_per_dir) {
 			/*
 			 * dir->basebuf gets reused by the traversal, but we
 			 * need fname to remain unchanged to ensure the src
 			 * member of each struct exclude correctly
 			 * back-references its source file.  Other invocations
 			 * of add_exclude_list provide stable strings, so we
-			 * strdup() and free() here in the caller.
+			 * strbuf_detach() and free() here in the caller.
 			 */
-			el->src = strdup(dir->basebuf);
-			add_excludes_from_file_to_list(dir->basebuf,
-					dir->basebuf, stk->baselen, el, 1);
+			struct strbuf sb = STRBUF_INIT;
+			strbuf_addbuf(&sb, &dir->basebuf);
+			strbuf_addstr(&sb, dir->exclude_per_dir);
+			el->src = strbuf_detach(&sb, NULL);
+			add_excludes_from_file_to_list(el->src, el->src,
+						       stk->baselen, el, 1);
 		}
 		dir->exclude_stack = stk;
 		current = stk->baselen;
 	}
-	dir->basebuf[baselen] = '\0';
+	strbuf_setlen(&dir->basebuf, baselen);
 }
 
 /*
@@ -1671,4 +1679,5 @@ void clear_directory(struct dir_struct *dir)
 		free(stk);
 		stk = prev;
 	}
+	strbuf_release(&dir->basebuf);
 }
diff --git a/dir.h b/dir.h
index 02e3710..6c45e9d 100644
--- a/dir.h
+++ b/dir.h
@@ -119,7 +119,7 @@ struct dir_struct {
 	 */
 	struct exclude_stack *exclude_stack;
 	struct exclude *exclude;
-	char basebuf[PATH_MAX];
+	struct strbuf basebuf;
 };
 
 /*
-- 
2.0.0.9646.g840d1f9.dirty

Re: [PATCH v1 1/3] dir.c: coding style fix

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=
>  <pclouds@gmail.com>
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Karsten Blees <blees@dcon.de>
> ---

Thanks for forwarding.   I'll fix-up the Yikes (see how these two
lines show the same name in a very different way), but how did you
produce the above?  Is there some fix we need in the toolchain that
produces patch e-mails?

>  dir.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index e65888d..3068ca8 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -557,8 +557,7 @@ int add_excludes_from_file_to_list(const char *fname,
>  			buf = xrealloc(buf, size+1);
>  			buf[size++] = '\n';
>  		}
> -	}
> -	else {
> +	} else {
>  		size = xsize_t(st.st_size);
>  		if (size == 0) {
>  			close(fd);
> @@ -793,9 +792,11 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
>  
>  	group = &dir->exclude_list_group[EXC_DIRS];
>  
> -	/* Pop the exclude lists from the EXCL_DIRS exclude_list_group
> +	/*
> +	 * Pop the exclude lists from the EXCL_DIRS exclude_list_group
>  	 * which originate from directories not in the prefix of the
> -	 * path being checked. */
> +	 * path being checked.
> +	 */
>  	while ((stk = dir->exclude_stack) != NULL) {
>  		if (stk->baselen <= baselen &&
>  		    !strncmp(dir->basebuf, base, stk->baselen))
> @@ -822,8 +823,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
>  		if (current < 0) {
>  			cp = base;
>  			current = 0;
> -		}
> -		else {
> +		} else {
>  			cp = strchr(base + current + 1, '/');
>  			if (!cp)
>  				die("oops in prep_exclude");

Re: [PATCH v1 1/3] dir.c: coding style fix

[Karsten Blees]

Am 15.07.2014 00:30, schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=
>>  <pclouds@gmail.com>
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> Signed-off-by: Karsten Blees <blees@dcon.de>
>> ---
> 
> Thanks for forwarding.   I'll fix-up the Yikes (see how these two
> lines show the same name in a very different way), but how did you
> produce the above?  Is there some fix we need in the toolchain that
> produces patch e-mails?
> 

Hmmm...I simply thought that this is how its supposed to work. Mail
headers can only contain US-ASCII, so the RFC 2047 Q-encoded-word
generated by git-format-patch looked good to me.

It seems git-mailinfo doesn't handle line breaks in header lines in
the mail body. I.e. if you remove the LF in the 'From:' line,
everything is fine, despite the Q-encoding.

Now, 'git-format-patch --from' seems to work around the problem, but
only for the 'From:' line. AFAICT there's no such option for
'Subject:', e.g. if you want to paste a patch after a scissors line,
you're on your own (see the example in the git-format-patch(1)
discussion section, with 'Subject:' both Q-encoded and line-wrapped).

Perhaps it should be clarified that git-format-patch output is not
suitable for pasting into mail clients? Or it should print headers
in plain text and let git-send-email handle the conversions?

Re: [PATCH v1 1/3] dir.c: coding style fix

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> Am 15.07.2014 00:30, schrieb Junio C Hamano:
>> Karsten Blees <karsten.blees@gmail.com> writes:
>> 
>>> From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=
>>>  <pclouds@gmail.com>
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> Signed-off-by: Karsten Blees <blees@dcon.de>
>>> ---
>> 
>> Thanks for forwarding.   I'll fix-up the Yikes (see how these two
>> lines show the same name in a very different way), but how did you
>> produce the above?  Is there some fix we need in the toolchain that
>> produces patch e-mails?
>> 
>
> Hmmm...I simply thought that this is how its supposed to work. Mail
> headers can only contain US-ASCII, so the RFC 2047 Q-encoded-word
> generated by git-format-patch looked good to me.

But that quoted one is *NOT* a mail header.  It is the first line of
the payload of your message, and should be in plain text just like
the remainder, e.g. S-o-b line that has the same name.

> Perhaps it should be clarified that git-format-patch output is not
> suitable for pasting into mail clients? Or it should print headers
> in plain text and let git-send-email handle the conversions?

If the former is missing, then we should definitely add it to the
documentation.  We often see new people pasting the "From " line
meant for /etc/magic and unwanted {From,Subject,Date}: in the body.

We may also want to add an option to tell it to produce an output
that is suitable for pasting into mail clients.  Hint, hint...