{"thread":{"id":"37125","subject":"[PATCH v1 1/3] dir.c: coding style fix","startedAt":"2014-07-14T09:47:11Z","lastAt":"2014-07-15T19:10:14Z","messageCount":6,"participants":["Karsten Blees","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"245977","messageId":"53C3A71F.1020101@gmail.com","threadId":"37125","inReplyTo":null,"subject":"[PATCH v1 1/3] dir.c: coding style fix","fromName":"Karsten Blees","fromEmail":"karsten.blees@gmail.com","sentAt":"2014-07-14T09:47:11Z","receivedAt":"2014-07-14T09:47:11Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n \n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Karsten Blees \n---\n dir.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex e65888d..3068ca8 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -557,8 +557,7 @@ int add_excludes_from_file_to_list(const char *fname,\n \t\t\tbuf = xrealloc(buf, size+1);\n \t\t\tbuf[size++] = '\\n';\n \t\t}\n-\t}\n-\telse {\n+\t} else {\n \t\tsize = xsize_t(st.st_size);\n \t\tif (size == 0) {\n \t\t\tclose(fd);\n@@ -793,9 +792,11 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \n \tgroup = &dir->exclude_list_group[EXC_DIRS];\n \n-\t/* Pop the exclude lists from the EXCL_DIRS exclude_list_group\n+\t/*\n+\t * Pop the exclude lists from the EXCL_DIRS exclude_list_group\n \t * which originate from directories not in the prefix of the\n-\t * path being checked. */\n+\t * path being checked.\n+\t */\n \twhile ((stk = dir->exclude_stack) != NULL) {\n \t\tif (stk->baselen <= baselen &&\n \t\t !strncmp(dir->basebuf, base, stk->baselen))\n@@ -822,8 +823,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t\tif (current < 0) {\n \t\t\tcp = base;\n \t\t\tcurrent = 0;\n-\t\t}\n-\t\telse {\n+\t\t} else {\n \t\t\tcp = strchr(base + current + 1, '/');\n \t\t\tif (!cp)\n \t\t\t\tdie(\"oops in prep_exclude\");\n-- \n2.0.0.9646.g840d1f9.dirty\n"},{"id":"245978","messageId":"53C3A753.5090700@gmail.com","threadId":"37125","inReplyTo":"53C3A71F.1020101@gmail.com","subject":"[PATCH v1 2/3] dir.h: move struct exclude declaration to top level","fromName":"Karsten Blees","fromEmail":"karsten.blees@gmail.com","sentAt":"2014-07-14T09:48:03Z","receivedAt":"2014-07-14T09:48:03Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n \n\nThere is no actual nested struct here. Move it out for clarity.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Karsten Blees \n---\n dir.h | 42 ++++++++++++++++++++++--------------------\n 1 file changed, 22 insertions(+), 20 deletions(-)\n\ndiff --git a/dir.h b/dir.h\nindex 55e5345..02e3710 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -15,6 +15,27 @@ struct dir_entry {\n #define EXC_FLAG_MUSTBEDIR 8\n #define EXC_FLAG_NEGATIVE 16\n \n+struct exclude {\n+\t/*\n+\t * This allows callers of last_exclude_matching() etc.\n+\t * to determine the origin of the matching pattern.\n+\t */\n+\tstruct exclude_list *el;\n+\n+\tconst char *pattern;\n+\tint patternlen;\n+\tint nowildcardlen;\n+\tconst char *base;\n+\tint baselen;\n+\tint flags;\n+\n+\t/*\n+\t * Counting starts from 1 for line numbers in ignore files,\n+\t * and from -1 decrementing for patterns from CLI args.\n+\t */\n+\tint srcpos;\n+};\n+\n /*\n * Each excludes file will be parsed into a fresh exclude_list which\n * is appended to the relevant exclude_list_group (either EXC_DIRS or\n@@ -32,26 +53,7 @@ struct exclude_list {\n \t/* origin of list, e.g. path to filename, or descriptive string */\n \tconst char *src;\n \n-\tstruct exclude {\n-\t\t/*\n-\t\t * This allows callers of last_exclude_matching() etc.\n-\t\t * to determine the origin of the matching pattern.\n-\t\t */\n-\t\tstruct exclude_list *el;\n-\n-\t\tconst char *pattern;\n-\t\tint patternlen;\n-\t\tint nowildcardlen;\n-\t\tconst char *base;\n-\t\tint baselen;\n-\t\tint flags;\n-\n-\t\t/*\n-\t\t * Counting starts from 1 for line numbers in ignore files,\n-\t\t * and from -1 decrementing for patterns from CLI args.\n-\t\t */\n-\t\tint srcpos;\n-\t} **excludes;\n+\tstruct exclude **excludes;\n };\n \n /*\n-- \n2.0.0.9646.g840d1f9.dirty\n"},{"id":"245979","messageId":"53C3A7DE.3020704@gmail.com","threadId":"37125","inReplyTo":"53C3A71F.1020101@gmail.com","subject":"[PATCH v1 3/3] prep_exclude: remove the artificial PATH_MAX limit","fromName":"Karsten Blees","fromEmail":"karsten.blees@gmail.com","sentAt":"2014-07-14T09:50:22Z","receivedAt":"2014-07-14T09:50:22Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n \n\nThis fixes a segfault in git-status with long paths on Windows,\nwhere PATH_MAX is only 260.\n\nThis also fixes the problem of silently ignoring .gitignore if the\nfull path exceeds PATH_MAX. Now add_excludes_from_file() will report\nif it gets ENAMETOOLONG.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Karsten Blees \n---\n dir.c | 47 ++++++++++++++++++++++++++++-------------------\n dir.h | 2 +-\n 2 files changed, 29 insertions(+), 20 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 3068ca8..fcb6872 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -799,12 +799,12 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t */\n \twhile ((stk = dir->exclude_stack) != NULL) {\n \t\tif (stk->baselen <= baselen &&\n-\t\t !strncmp(dir->basebuf, base, stk->baselen))\n+\t\t !strncmp(dir->basebuf.buf, base, stk->baselen))\n \t\t\tbreak;\n \t\tel = &group->el[dir->exclude_stack->exclude_ix];\n \t\tdir->exclude_stack = stk->prev;\n \t\tdir->exclude = NULL;\n-\t\tfree((char *)el->src); /* see strdup() below */\n+\t\tfree((char *)el->src); /* see strbuf_detach() below */\n \t\tclear_exclude_list(el);\n \t\tfree(stk);\n \t\tgroup->nr--;\n@@ -814,8 +814,17 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \tif (dir->exclude)\n \t\treturn;\n \n+\t/*\n+\t * Lazy initialization. All call sites currently just\n+\t * memset(dir, 0, sizeof(*dir)) before use. Changing all of\n+\t * them seems lots of work for little benefit.\n+\t */\n+\tif (!dir->basebuf.buf)\n+\t\tstrbuf_init(&dir->basebuf, PATH_MAX);\n+\n \t/* Read from the parent directories and push them down. */\n \tcurrent = stk ? stk->baselen : -1;\n+\tstrbuf_setlen(&dir->basebuf, current < 0 ? 0 : current);\n \twhile (current < baselen) {\n \t\tstruct exclude_stack *stk = xcalloc(1, sizeof(*stk));\n \t\tconst char *cp;\n@@ -833,48 +842,47 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t\tstk->baselen = cp - base;\n \t\tstk->exclude_ix = group->nr;\n \t\tel = add_exclude_list(dir, EXC_DIRS, NULL);\n-\t\tmemcpy(dir->basebuf + current, base + current,\n-\t\t stk->baselen - current);\n+\t\tstrbuf_add(&dir->basebuf, base + current, stk->baselen - current);\n+\t\tassert(stk->baselen == dir->basebuf.len);\n \n \t\t/* Abort if the directory is excluded */\n \t\tif (stk->baselen) {\n \t\t\tint dt = DT_DIR;\n-\t\t\tdir->basebuf[stk->baselen - 1] = 0;\n+\t\t\tdir->basebuf.buf[stk->baselen - 1] = 0;\n \t\t\tdir->exclude = last_exclude_matching_from_lists(dir,\n-\t\t\t\tdir->basebuf, stk->baselen - 1,\n-\t\t\t\tdir->basebuf + current, &dt);\n-\t\t\tdir->basebuf[stk->baselen - 1] = '/';\n+\t\t\t\tdir->basebuf.buf, stk->baselen - 1,\n+\t\t\t\tdir->basebuf.buf + current, &dt);\n+\t\t\tdir->basebuf.buf[stk->baselen - 1] = '/';\n \t\t\tif (dir->exclude &&\n \t\t\t dir->exclude->flags & EXC_FLAG_NEGATIVE)\n \t\t\t\tdir->exclude = NULL;\n \t\t\tif (dir->exclude) {\n-\t\t\t\tdir->basebuf[stk->baselen] = 0;\n \t\t\t\tdir->exclude_stack = stk;\n \t\t\t\treturn;\n \t\t\t}\n \t\t}\n \n-\t\t/* Try to read per-directory file unless path is too long */\n-\t\tif (dir->exclude_per_dir &&\n-\t\t stk->baselen + strlen(dir->exclude_per_dir) < PATH_MAX) {\n-\t\t\tstrcpy(dir->basebuf + stk->baselen,\n-\t\t\t\t\tdir->exclude_per_dir);\n+\t\t/* Try to read per-directory file */\n+\t\tif (dir->exclude_per_dir) {\n \t\t\t/*\n \t\t\t * dir->basebuf gets reused by the traversal, but we\n \t\t\t * need fname to remain unchanged to ensure the src\n \t\t\t * member of each struct exclude correctly\n \t\t\t * back-references its source file. Other invocations\n \t\t\t * of add_exclude_list provide stable strings, so we\n-\t\t\t * strdup() and free() here in the caller.\n+\t\t\t * strbuf_detach() and free() here in the caller.\n \t\t\t */\n-\t\t\tel->src = strdup(dir->basebuf);\n-\t\t\tadd_excludes_from_file_to_list(dir->basebuf,\n-\t\t\t\t\tdir->basebuf, stk->baselen, el, 1);\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tstrbuf_addbuf(&sb, &dir->basebuf);\n+\t\t\tstrbuf_addstr(&sb, dir->exclude_per_dir);\n+\t\t\tel->src = strbuf_detach(&sb, NULL);\n+\t\t\tadd_excludes_from_file_to_list(el->src, el->src,\n+\t\t\t\t\t\t stk->baselen, el, 1);\n \t\t}\n \t\tdir->exclude_stack = stk;\n \t\tcurrent = stk->baselen;\n \t}\n-\tdir->basebuf[baselen] = '\\0';\n+\tstrbuf_setlen(&dir->basebuf, baselen);\n }\n \n /*\n@@ -1671,4 +1679,5 @@ void clear_directory(struct dir_struct *dir)\n \t\tfree(stk);\n \t\tstk = prev;\n \t}\n+\tstrbuf_release(&dir->basebuf);\n }\ndiff --git a/dir.h b/dir.h\nindex 02e3710..6c45e9d 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -119,7 +119,7 @@ struct dir_struct {\n \t */\n \tstruct exclude_stack *exclude_stack;\n \tstruct exclude *exclude;\n-\tchar basebuf[PATH_MAX];\n+\tstruct strbuf basebuf;\n };\n \n /*\n-- \n2.0.0.9646.g840d1f9.dirty\n"},{"id":"246038","messageId":"xmqq4myjy3ms.fsf@gitster.dls.corp.google.com","threadId":"37125","inReplyTo":"53C3A71F.1020101@gmail.com","subject":"Re: [PATCH v1 1/3] dir.c: coding style fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-14T22:30:51Z","receivedAt":"2014-07-14T22:30:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karsten Blees writes:\n\n> From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n> \n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> Signed-off-by: Karsten Blees \n> ---\n\nThanks for forwarding. I'll fix-up the Yikes (see how these two\nlines show the same name in a very different way), but how did you\nproduce the above? Is there some fix we need in the toolchain that\nproduces patch e-mails?\n\n> dir.c | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index e65888d..3068ca8 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -557,8 +557,7 @@ int add_excludes_from_file_to_list(const char *fname,\n> \t\t\tbuf = xrealloc(buf, size+1);\n> \t\t\tbuf[size++] = '\\n';\n> \t\t}\n> -\t}\n> -\telse {\n> +\t} else {\n> \t\tsize = xsize_t(st.st_size);\n> \t\tif (size == 0) {\n> \t\t\tclose(fd);\n> @@ -793,9 +792,11 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n> \n> \tgroup = &dir->exclude_list_group[EXC_DIRS];\n> \n> -\t/* Pop the exclude lists from the EXCL_DIRS exclude_list_group\n> +\t/*\n> +\t * Pop the exclude lists from the EXCL_DIRS exclude_list_group\n> \t * which originate from directories not in the prefix of the\n> -\t * path being checked. */\n> +\t * path being checked.\n> +\t */\n> \twhile ((stk = dir->exclude_stack) != NULL) {\n> \t\tif (stk->baselen <= baselen &&\n> \t\t !strncmp(dir->basebuf, base, stk->baselen))\n> @@ -822,8 +823,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n> \t\tif (current < 0) {\n> \t\t\tcp = base;\n> \t\t\tcurrent = 0;\n> -\t\t}\n> -\t\telse {\n> +\t\t} else {\n> \t\t\tcp = strchr(base + current + 1, '/');\n> \t\t\tif (!cp)\n> \t\t\t\tdie(\"oops in prep_exclude\");\n"},{"id":"246077","messageId":"53C579B4.2050506@gmail.com","threadId":"37125","inReplyTo":"xmqq4myjy3ms.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v1 1/3] dir.c: coding style fix","fromName":"Karsten Blees","fromEmail":"karsten.blees@gmail.com","sentAt":"2014-07-15T18:57:56Z","receivedAt":"2014-07-15T18:57:56Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"Am 15.07.2014 00:30, schrieb Junio C Hamano:\n> Karsten Blees writes:\n> \n>> From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n>> \n>>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>> Signed-off-by: Karsten Blees \n>> ---\n> \n> Thanks for forwarding. I'll fix-up the Yikes (see how these two\n> lines show the same name in a very different way), but how did you\n> produce the above? Is there some fix we need in the toolchain that\n> produces patch e-mails?\n> \n\nHmmm...I simply thought that this is how its supposed to work. Mail\nheaders can only contain US-ASCII, so the RFC 2047 Q-encoded-word\ngenerated by git-format-patch looked good to me.\n\nIt seems git-mailinfo doesn't handle line breaks in header lines in\nthe mail body. I.e. if you remove the LF in the 'From:' line,\neverything is fine, despite the Q-encoding.\n\nNow, 'git-format-patch --from' seems to work around the problem, but\nonly for the 'From:' line. AFAICT there's no such option for\n'Subject:', e.g. if you want to paste a patch after a scissors line,\nyou're on your own (see the example in the git-format-patch(1)\ndiscussion section, with 'Subject:' both Q-encoded and line-wrapped).\n\nPerhaps it should be clarified that git-format-patch output is not\nsuitable for pasting into mail clients? Or it should print headers\nin plain text and let git-send-email handle the conversions?\n"},{"id":"246078","messageId":"xmqqpph6v3op.fsf@gitster.dls.corp.google.com","threadId":"37125","inReplyTo":"53C579B4.2050506@gmail.com","subject":"Re: [PATCH v1 1/3] dir.c: coding style fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-15T19:10:14Z","receivedAt":"2014-07-15T19:10:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karsten Blees writes:\n\n> Am 15.07.2014 00:30, schrieb Junio C Hamano:\n>> Karsten Blees writes:\n>> \n>>> From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?=\n>>> \n>>>\n>>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>>> Signed-off-by: Karsten Blees \n>>> ---\n>> \n>> Thanks for forwarding. I'll fix-up the Yikes (see how these two\n>> lines show the same name in a very different way), but how did you\n>> produce the above? Is there some fix we need in the toolchain that\n>> produces patch e-mails?\n>> \n>\n> Hmmm...I simply thought that this is how its supposed to work. Mail\n> headers can only contain US-ASCII, so the RFC 2047 Q-encoded-word\n> generated by git-format-patch looked good to me.\n\nBut that quoted one is *NOT* a mail header. It is the first line of\nthe payload of your message, and should be in plain text just like\nthe remainder, e.g. S-o-b line that has the same name.\n\n> Perhaps it should be clarified that git-format-patch output is not\n> suitable for pasting into mail clients? Or it should print headers\n> in plain text and let git-send-email handle the conversions?\n\nIf the former is missing, then we should definitely add it to the\ndocumentation. We often see new people pasting the \"From \" line\nmeant for /etc/magic and unwanted {From,Subject,Date}: in the body.\n\nWe may also want to add an option to tell it to produce an output\nthat is suitable for pasting into mail clients. Hint, hint...\n"}]}