{"thread":{"id":"7118","subject":"[PATCH 2/5] add the ability to select more email header fields to output","startedAt":"2007-03-06T21:57:39Z","lastAt":"2007-03-07T17:17:44Z","messageCount":8,"participants":["Don Zickus","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"36481","messageId":"1173218263315-git-send-email-dzickus@redhat.com","threadId":"7118","inReplyTo":null,"subject":"[PATCH 1/5] builtin-mailinfo.c infrastrcture changes","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2007-03-06T21:57:39Z","receivedAt":"2007-03-06T21:57:39Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"I am working on a project that required parsing through regular mboxes that\ndidn't necessarily have patches embedded in them.  I started by creating my\nown modified copy of git-am and working from there.  Very quickly, I noticed\ngit-mailinfo wasn't able to handle a big chunk of my email.\n\nAfter hacking up numerous solutions and running into more limitations, I\ndecided it was just easier to rewrite a big chunk of it.  The following\npatch has a bunch of fixes and features that I needed in order for me do\nwhat I wanted.\n\nNote: I'm didn't follow any email rfc papers but I don't think any of the\nchanges I did required much knowledge (besides the boundary stuff).\n\nSorry for the large patchset.  It was too hard to break it up without\ncreating a bunch of intermediate throwaway changes.  If it is too large, I\ncan try to break it down some.\n\nList of major changes/fixes:\n- can't create empty patch files fix\n- empty patch files don't fail, this failure will come inside git-am\n- multipart boundaries are now handled\n- only output inbody headers if a patch exists otherwise assume those\nheaders are part of the reply and instead output the original headers\n- decode and filter base64 patches correctly\n- various other accidental fixes\n\nI believe I didn't break any existing functionality or compatibility (other\nthan what I describe above, which is really only the empty patch file).\n\nI tested this through various mailing list archives and everything seemed to\nparse correctly (a couple thousand emails).\n\nAny comments or feedback would be great.\n\nSigned-off-by: Don Zickus <dzickus@redhat.com>\n---\n builtin-mailinfo.c |  514 +++++++++++++++++++++++++++-------------------------\n git-am.sh          |    1 +\n 2 files changed, 269 insertions(+), 246 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 766a37e..7456d8a 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -11,19 +11,19 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;\n static int keep_subject;\n static const char *metainfo_charset;\n static char line[1000];\n-static char date[1000];\n static char name[1000];\n static char email[1000];\n-static char subject[1000];\n \n static enum  {\n \tTE_DONTCARE, TE_QP, TE_BASE64,\n } transfer_encoding;\n-static char charset[256];\n+static enum  {\n+\tTYPE_TEXT, TYPE_OTHER,\n+} message_type;\n \n-static char multipart_boundary[1000];\n-static int multipart_boundary_len;\n+static char charset[256];\n static int patch_lines;\n+static char **p_hdr_data, **s_hdr_data;\n \n static char *sanity_check(char *name, char *email)\n {\n@@ -137,15 +137,13 @@ static int handle_from(char *in_line)\n \treturn 1;\n }\n \n-static int handle_date(char *line)\n+static int handle_header(char *line, char *data, int ofs)\n {\n-\tstrcpy(date, line);\n-\treturn 0;\n-}\n+\tif (!line || !data)\n+\t\treturn 1;\n+\n+\tstrcpy(data, line+ofs);\n \n-static int handle_subject(char *line)\n-{\n-\tstrcpy(subject, line);\n \treturn 0;\n }\n \n@@ -177,17 +175,35 @@ static int slurp_attr(const char *line, const char *name, char *attr)\n \treturn 1;\n }\n \n-static int handle_subcontent_type(char *line)\n+struct content_type {\n+\tchar *boundary;\n+\tint boundary_len;\n+};\n+\n+static struct content_type content[]={\n+\t{ NULL },\n+\t{ NULL },\n+\t{ NULL },\n+\t{ NULL },\n+\t{ NULL },\n+};\n+\n+static struct content_type *content_top = content;\n+\n+static int handle_content_type(char *line)\n {\n-\t/* We do not want to mess with boundary.  Note that we do not\n-\t * handle nested multipart.\n-\t */\n-\tif (strcasestr(line, \"boundary=\")) {\n-\t\tfprintf(stderr, \"Not handling nested multipart message.\\n\");\n-\t\texit(1);\n+\tchar boundary[256];\n+\n+\tif (strcasestr(line, \"text/\") < 0)\n+\t\t message_type = TYPE_OTHER;\n+\tif (slurp_attr(line, \"boundary=\", boundary + 2)) {\n+\t\tmemcpy(boundary, \"--\", 2);\n+\t\tcontent_top++;\n+\t\tcontent_top->boundary_len = strlen(boundary);\n+\t\tcontent_top->boundary = xmalloc(content_top->boundary_len+1);\n+\t\tstrcpy(content_top->boundary, boundary);\n \t}\n-\tslurp_attr(line, \"charset=\", charset);\n-\tif (*charset) {\n+\tif (slurp_attr(line, \"charset=\", charset)) {\n \t\tint i, c;\n \t\tfor (i = 0; (c = charset[i]) != 0; i++)\n \t\t\tcharset[i] = tolower(c);\n@@ -195,17 +211,6 @@ static int handle_subcontent_type(char *line)\n \treturn 0;\n }\n \n-static int handle_content_type(char *line)\n-{\n-\t*multipart_boundary = 0;\n-\tif (slurp_attr(line, \"boundary=\", multipart_boundary + 2)) {\n-\t\tmemcpy(multipart_boundary, \"--\", 2);\n-\t\tmultipart_boundary_len = strlen(multipart_boundary);\n-\t}\n-\tslurp_attr(line, \"charset=\", charset);\n-\treturn 0;\n-}\n-\n static int handle_content_transfer_encoding(char *line)\n {\n \tif (strcasestr(line, \"base64\"))\n@@ -219,7 +224,7 @@ static int handle_content_transfer_encoding(char *line)\n \n static int is_multipart_boundary(const char *line)\n {\n-\treturn (!memcmp(line, multipart_boundary, multipart_boundary_len));\n+\treturn (!memcmp(line, content_top->boundary, content_top->boundary_len));\n }\n \n static int eatspace(char *line)\n@@ -230,62 +235,6 @@ static int eatspace(char *line)\n \treturn len;\n }\n \n-#define SEEN_FROM 01\n-#define SEEN_DATE 02\n-#define SEEN_SUBJECT 04\n-#define SEEN_BOGUS_UNIX_FROM 010\n-#define SEEN_PREFIX  020\n-\n-/* First lines of body can have From:, Date:, and Subject: or empty */\n-static void handle_inbody_header(int *seen, char *line)\n-{\n-\tif (*seen & SEEN_PREFIX)\n-\t\treturn;\n-\tif (isspace(*line)) {\n-\t\tchar *cp;\n-\t\tfor (cp = line + 1; *cp; cp++) {\n-\t\t\tif (!isspace(*cp))\n-\t\t\t\tbreak;\n-\t\t}\n-\t\tif (!*cp)\n-\t\t\treturn;\n-\t}\n-\tif (!memcmp(\">From\", line, 5) && isspace(line[5])) {\n-\t\tif (!(*seen & SEEN_BOGUS_UNIX_FROM)) {\n-\t\t\t*seen |= SEEN_BOGUS_UNIX_FROM;\n-\t\t\treturn;\n-\t\t}\n-\t}\n-\tif (!memcmp(\"From:\", line, 5) && isspace(line[5])) {\n-\t\tif (!(*seen & SEEN_FROM) && handle_from(line+6)) {\n-\t\t\t*seen |= SEEN_FROM;\n-\t\t\treturn;\n-\t\t}\n-\t}\n-\tif (!memcmp(\"Date:\", line, 5) && isspace(line[5])) {\n-\t\tif (!(*seen & SEEN_DATE)) {\n-\t\t\thandle_date(line+6);\n-\t\t\t*seen |= SEEN_DATE;\n-\t\t\treturn;\n-\t\t}\n-\t}\n-\tif (!memcmp(\"Subject:\", line, 8) && isspace(line[8])) {\n-\t\tif (!(*seen & SEEN_SUBJECT)) {\n-\t\t\thandle_subject(line+9);\n-\t\t\t*seen |= SEEN_SUBJECT;\n-\t\t\treturn;\n-\t\t}\n-\t}\n-\tif (!memcmp(\"[PATCH]\", line, 7) && isspace(line[7])) {\n-\t\tif (!(*seen & SEEN_SUBJECT)) {\n-\t\t\thandle_subject(line);\n-\t\t\t*seen |= SEEN_SUBJECT;\n-\t\t\treturn;\n-\t\t}\n-\t}\n-\t*seen |= SEEN_PREFIX;\n-}\n-\n static char *cleanup_subject(char *subject)\n {\n \tif (keep_subject)\n@@ -341,57 +290,65 @@ static void cleanup_space(char *buf)\n }\n \n static void decode_header(char *it);\n-typedef int (*header_fn_t)(char *);\n-struct header_def {\n-\tconst char *name;\n-\theader_fn_t func;\n-\tint namelen;\n+static char *header[10] = {\n+\t\"From\",\n+\t\"Subject\",\n+\t\"Date\",\n+\tNULL,\n+\tNULL,\n+\tNULL,\n };\n \n-static void check_header(char *line, struct header_def *header)\n+static int check_header(char *line, char **hdr_data)\n {\n \tint i;\n \n-\tif (header[0].namelen <= 0) {\n-\t\tfor (i = 0; header[i].name; i++)\n-\t\t\theader[i].namelen = strlen(header[i].name);\n-\t}\n-\tfor (i = 0; header[i].name; i++) {\n-\t\tint len = header[i].namelen;\n-\t\tif (!strncasecmp(line, header[i].name, len) &&\n+\t/* search for the interesting parts */\n+\tfor (i = 0; header[i]; i++) {\n+\t\tint len = strlen(header[i]);\n+\t\tif (!hdr_data[i] &&\n+\t\t    !strncasecmp(line, header[i], len) &&\n \t\t    line[len] == ':' && isspace(line[len + 1])) {\n \t\t\t/* Unwrap inline B and Q encoding, and optionally\n \t\t\t * normalize the meta information to utf8.\n \t\t\t */\n \t\t\tdecode_header(line + len + 2);\n-\t\t\theader[i].func(line + len + 2);\n-\t\t\tbreak;\n+\t\t\thdr_data[i] = xmalloc(1000 * sizeof(char));\n+\t\t\tif (! handle_header(line, hdr_data[i], len + 2)) {\n+\t\t\t\treturn 1;\n+\t\t\t}\n \t\t}\n \t}\n-}\n \n-static void check_subheader_line(char *line)\n-{\n-\tstatic struct header_def header[] = {\n-\t\t{ \"Content-Type\", handle_subcontent_type },\n-\t\t{ \"Content-Transfer-Encoding\",\n-\t\t  handle_content_transfer_encoding },\n-\t\t{ NULL },\n-\t};\n-\tcheck_header(line, header);\n-}\n-static void check_header_line(char *line)\n-{\n-\tstatic struct header_def header[] = {\n-\t\t{ \"From\", handle_from },\n-\t\t{ \"Date\", handle_date },\n-\t\t{ \"Subject\", handle_subject },\n-\t\t{ \"Content-Type\", handle_content_type },\n-\t\t{ \"Content-Transfer-Encoding\",\n-\t\t  handle_content_transfer_encoding },\n-\t\t{ NULL },\n-\t};\n-\tcheck_header(line, header);\n+\t/* Content stuff */\n+\tif (!strncasecmp(line, \"Content-Type: \", 14)) {\n+\t\tdecode_header(line + 14);\n+\t\tif (! handle_content_type(line)) {\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\tif (!strncasecmp(line, \"Content-Transfer-Encoding: \", 27)) {\n+\t\tdecode_header(line + 27);\n+\t\tif (! handle_content_transfer_encoding(line)) {\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\n+\t/* for inbody stuff */\n+\tif (!memcmp(\">From\", line, 5) && isspace(line[5]))\n+\t\treturn 1;\n+\tif (!memcmp(\"[PATCH]\", line, 7) && isspace(line[7])) {\n+\t\tfor (i=0; header[i]; i++) {\n+\t\t\tif (!memcmp(\"Subject: \", header[i], 9)) {\n+\t\t\t\tif (! handle_header(line, hdr_data[i], 0)) {\n+\t\t\t\t\treturn 1;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t/* no match */\n+\treturn 0;\n }\n \n static int is_rfc2822_header(char *line)\n@@ -647,147 +604,214 @@ static void decode_transfer_encoding(char *line)\n \t}\n }\n \n-static void handle_info(void)\n+static int handle_filter(char *line);\n+\n+static int find_boundary(void)\n {\n-\tchar *sub;\n+\twhile(fgets(line, sizeof(line), fin) != NULL) {\n+\t\tif (is_multipart_boundary(line))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int handle_boundary(void)\n+{\n+again:\n+\tif (!memcmp(line+content_top->boundary_len, \"--\", 2)) {\n+\t\t/* we hit an end boundary */\n+\t\t/* pop the current boundary off the stack */\n+\t\tfree(content_top->boundary);\n+\t\tcontent_top--;\n+\t\thandle_filter(\"\\n\");\n+\n+\t\t/* skip to the next boundary */\n+\t\tif (!find_boundary())\n+\t\t\treturn 0;\n+\t\tgoto again;\n+\t}\n+\n+\t/* set some defaults */\n+\ttransfer_encoding = TE_DONTCARE;\n+\tcharset[0] = 0;\n+\tmessage_type = TYPE_TEXT;\n \n-\tsub = cleanup_subject(subject);\n-\tcleanup_space(name);\n-\tcleanup_space(date);\n-\tcleanup_space(email);\n-\tcleanup_space(sub);\n+\t/* slurp in this section's info */\n+\twhile (read_one_header_line(line, sizeof(line), fin))\n+\t\tcheck_header(line, p_hdr_data);\n \n-\tfprintf(fout, \"Author: %s\\nEmail: %s\\nSubject: %s\\nDate: %s\\n\\n\",\n-\t       name, email, sub, date);\n+\t/* eat the blank line after section info */\n+\treturn (fgets(line, sizeof(line), fin) != NULL);\n }\n \n-/* We are inside message body and have read line[] already.\n- * Spit out the commit log.\n- */\n-static int handle_commit_msg(int *seen)\n+static int handle_commit_msg(char *line)\n {\n+\tstatic int still_looking=1;\n+\n \tif (!cmitmsg)\n \t\treturn 0;\n-\tdo {\n-\t\tif (!memcmp(\"diff -\", line, 6) ||\n-\t\t    !memcmp(\"---\", line, 3) ||\n-\t\t    !memcmp(\"Index: \", line, 7))\n-\t\t\tbreak;\n-\t\tif ((multipart_boundary[0] && is_multipart_boundary(line))) {\n-\t\t\t/* We come here when the first part had only\n-\t\t\t * the commit message without any patch.  We\n-\t\t\t * pretend we have not seen this line yet, and\n-\t\t\t * go back to the loop.\n-\t\t\t */\n-\t\t\treturn 1;\n-\t\t}\n \n-\t\t/* Unwrap transfer encoding and optionally\n-\t\t * normalize the log message to UTF-8.\n-\t\t */\n-\t\tdecode_transfer_encoding(line);\n-\t\tif (metainfo_charset)\n-\t\t\tconvert_to_utf8(line, charset);\n+\tif (still_looking) {\n+\t\tchar *cp=line;\n+\t\tif (isspace(*line)) {\n+\t\t\tfor (cp = line + 1; *cp; cp++) {\n+\t\t\t\tif (!isspace(*cp))\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (!*cp)\n+\t\t\t\treturn 0;\n+\t\t}\n+\t\tif ((still_looking = check_header(cp, s_hdr_data)) != 0)\n+\t\t\treturn 0;\n+\t}\n \n-\t\thandle_inbody_header(seen, line);\n-\t\tif (!(*seen & SEEN_PREFIX))\n-\t\t\tcontinue;\n+\tif (!memcmp(\"diff -\", line, 6) ||\n+\t    !memcmp(\"---\", line, 3) ||\n+\t    !memcmp(\"Index: \", line, 7)) {\n+\t\tfclose(cmitmsg);\n+\t\tcmitmsg = NULL;\n+\t\treturn 1;\n+\t}\n \n-\t\tfputs(line, cmitmsg);\n-\t} while (fgets(line, sizeof(line), fin) != NULL);\n-\tfclose(cmitmsg);\n-\tcmitmsg = NULL;\n+\tfputs(line, cmitmsg);\n \treturn 0;\n }\n \n-/* We have done the commit message and have the first\n- * line of the patch in line[].\n- */\n-static void handle_patch(void)\n+static int handle_patch(char *line)\n {\n-\tdo {\n-\t\tif (multipart_boundary[0] && is_multipart_boundary(line))\n-\t\t\tbreak;\n-\t\t/* Only unwrap transfer encoding but otherwise do not\n-\t\t * do anything.  We do *NOT* want UTF-8 conversion\n-\t\t * here; we are dealing with the user payload.\n-\t\t */\n-\t\tdecode_transfer_encoding(line);\n-\t\tfputs(line, patchfile);\n-\t\tpatch_lines++;\n-\t} while (fgets(line, sizeof(line), fin) != NULL);\n+\tfputs(line, patchfile);\n+\tpatch_lines++;\n+\treturn 0;\n }\n \n-/* multipart boundary and transfer encoding are set up for us, and we\n- * are at the end of the sub header.  do equivalent of handle_body up\n- * to the next boundary without closing patchfile --- we will expect\n- * that the first part to contain commit message and a patch, and\n- * handle other parts as pure patches.\n- */\n-static int handle_multipart_one_part(int *seen)\n+static int handle_filter(char *line)\n {\n-\tint n = 0;\n+\tstatic int filter=0;\n \n-\twhile (fgets(line, sizeof(line), fin) != NULL) {\n-\tagain:\n-\t\tn++;\n-\t\tif (is_multipart_boundary(line))\n+\t/* filter tells us which part we left off on\n+\t * a non-zero return indicates we hit a filter point\n+\t */\n+\tswitch (filter) {\n+\tcase 0:\n+\t\tif (!handle_commit_msg(line))\n \t\t\tbreak;\n-\t\tif (handle_commit_msg(seen))\n-\t\t\tgoto again;\n-\t\thandle_patch();\n-\t\tbreak;\n+\t\tfilter++;\n+\tcase 1:\n+\t\tif (!handle_patch(line))\n+\t\t\tbreak;\n+\t\tfilter++;\n+\tdefault:\n+\t\treturn 1;\n \t}\n-\tif (n == 0)\n-\t\treturn -1;\n+\n \treturn 0;\n }\n \n-static void handle_multipart_body(void)\n+static void handle_body(void)\n {\n-\tint seen = 0;\n-\tint part_num = 0;\n+\tint rc=0;\n+\tstatic char newline[2000];\n+\tstatic char *np=newline;\n \n \t/* Skip up to the first boundary */\n-\twhile (fgets(line, sizeof(line), fin) != NULL)\n-\t\tif (is_multipart_boundary(line)) {\n-\t\t\tpart_num = 1;\n+\tif (content_top->boundary) {\n+\t\tif (!find_boundary())\n+\t\t\treturn;\n+\t}\n+\n+\tdo {\n+\t\t/* process any boundary lines */\n+\t\tif (content_top->boundary && is_multipart_boundary(line)) {\n+\t\t\t/* flush any leftover */\n+\t\t\tif ((transfer_encoding == TE_BASE64)  &&\n+\t\t\t    (np != newline)) {\n+\t\t\t\thandle_filter(newline);\n+\t\t\t}\n+\t\t\tif (!handle_boundary())\n+\t\t\t\treturn;\n+\t\t}\n+\n+\t\t/* Unwrap transfer encoding and optionally\n+\t\t * normalize the log message to UTF-8.\n+\t\t */\n+\t\tdecode_transfer_encoding(line);\n+\t\tif (metainfo_charset)\n+\t\t\tconvert_to_utf8(line, charset);\n+\n+\t\tswitch (transfer_encoding) {\n+\t\tcase TE_BASE64:\n+\t\t{\n+\t\t\t/* binary data most likely doesn't have newlines */\n+\t\t\tif (message_type != TYPE_TEXT) {\n+\t\t\t\trc=handle_filter(line);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\t/* this is a decoded line that may contain\n+\t\t\t * multiple new lines.  Pass only one chunk\n+\t\t\t * at a time to handle_filter()\n+\t\t\t */\n+\n+\t\t\tchar *op=line;\n+\n+\t\t\tdo {\n+\t\t\t\twhile (*op != '\\n' && *op != 0)\n+\t\t\t\t\t*np++ = *op++;\n+\t\t\t\t*np = *op;\n+\t\t\t\tif (*np != 0) {\n+\t\t\t\t\t/* should be sitting on a new line */\n+\t\t\t\t\t*(++np) = 0;\n+\t\t\t\t\top++;\n+\t\t\t\t\trc=handle_filter(newline);\n+\t\t\t\t\tnp=newline;\n+\t\t\t\t}\n+\t\t\t} while (*op != 0);\n+\t\t\t/* the partial chunk is saved in newline and\n+\t\t\t * will be appended by the next iteration of fgets\n+\t\t\t */\n \t\t\tbreak;\n \t\t}\n-\tif (!part_num)\n-\t\treturn;\n-\t/* We are on boundary line.  Start slurping the subhead. */\n-\twhile (1) {\n-\t\tint hdr = read_one_header_line(line, sizeof(line), fin);\n-\t\tif (!hdr) {\n-\t\t\tif (handle_multipart_one_part(&seen) < 0)\n-\t\t\t\treturn;\n-\t\t\t/* Reset per part headers */\n-\t\t\ttransfer_encoding = TE_DONTCARE;\n-\t\t\tcharset[0] = 0;\n+\t\tdefault:\n+\t\t\trc=handle_filter(line);\n \t\t}\n-\t\telse\n-\t\t\tcheck_subheader_line(line);\n-\t}\n-\tfclose(patchfile);\n-\tif (!patch_lines) {\n-\t\tfprintf(stderr, \"No patch found\\n\");\n-\t\texit(1);\n-\t}\n+\t\tif (rc)\n+\t\t\t/* nothing left to filter */\n+\t\t\tbreak;\n+\t} while (fgets(line, sizeof(line), fin));\n+\n+\treturn;\n }\n \n-/* Non multipart message */\n-static void handle_body(void)\n+static void handle_info(void)\n {\n-\tint seen = 0;\n-\n-\thandle_commit_msg(&seen);\n-\thandle_patch();\n-\tfclose(patchfile);\n-\tif (!patch_lines) {\n-\t\tfprintf(stderr, \"No patch found\\n\");\n-\t\texit(1);\n+\tchar *sub;\n+\tchar *hdr;\n+\tint i;\n+\n+\tfor (i=0; header[i]; i++) {\n+\n+\t\t/* only print inbody headers if we output a patch file */\n+\t\tif (patch_lines && s_hdr_data[i])\n+\t\t\thdr=s_hdr_data[i];\n+\t\telse if (p_hdr_data[i])\n+\t\t\thdr=p_hdr_data[i];\n+\t\telse\n+\t\t\tcontinue;\n+\n+\t\tif (!memcmp(header[i], \"Subject\", 7)) {\n+\t\t\tsub = cleanup_subject(hdr);\n+\t\t\tcleanup_space(sub);\n+\t\t\tfprintf(fout, \"Subject: %s\\n\", sub);\n+\t\t} else if (!memcmp(header[i], \"From\", 4)) {\n+\t\t\thandle_from(hdr);\n+\t\t\tfprintf(fout, \"Author: %s\\n\", name);\n+\t\t\tfprintf(fout, \"Email: %s\\n\", email);\n+\t\t} else {\n+\t\t\tcleanup_space(hdr);\n+\t\t\tfprintf(fout, \"%s: %s\\n\", header[i], hdr);\n+\t\t}\n \t}\n+\tfprintf(fout, \"\\n\");\n }\n \n int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,\n@@ -809,18 +833,16 @@ int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,\n \t\tfclose(cmitmsg);\n \t\treturn -1;\n \t}\n-\twhile (1) {\n-\t\tint hdr = read_one_header_line(line, sizeof(line), fin);\n-\t\tif (!hdr) {\n-\t\t\tif (multipart_boundary[0])\n-\t\t\t\thandle_multipart_body();\n-\t\t\telse\n-\t\t\t\thandle_body();\n-\t\t\thandle_info();\n-\t\t\tbreak;\n-\t\t}\n-\t\tcheck_header_line(line);\n-\t}\n+\n+\tp_hdr_data = xcalloc(10, sizeof(char *));\n+\ts_hdr_data = xcalloc(10, sizeof(char *));\n+\n+\t/* process the email header */\n+\twhile (read_one_header_line(line, sizeof(line), fin))\n+\t\tcheck_header(line, p_hdr_data);\n+\n+\thandle_body();\n+\thandle_info();\n \n \treturn 0;\n }\ndiff --git a/git-am.sh b/git-am.sh\nindex 2c73d11..8fa466a 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -290,6 +290,7 @@ do\n \t\tgit-mailinfo $keep $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n \t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n \t\t\tstop_here $this\n+\t\ttest -s $dotest/patch || stop_here $this\n \t\tgit-stripspace < \"$dotest/msg\" > \"$dotest/msg-clean\"\n \t\t;;\n \tesac\n-- \n1.5.0.2.212.gd52f-dirty\n"},{"id":"36477","messageId":"11732182643385-git-send-email-dzickus@redhat.com","threadId":"7118","inReplyTo":"1173218263315-git-send-email-dzickus@redhat.com","subject":"[PATCH 2/5] add the ability to select more email header fields to output","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2007-03-06T21:57:40Z","receivedAt":"2007-03-06T21:57:40Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"This is useful when scripts need more than just the basic email headers to\nparse.  By specifying the \"-x=\" option, one can search and output any header\nfield they want.\n\nSigned-off-by: Don Zickus <dzickus@redhat.com>\n---\n builtin-mailinfo.c |   10 ++++++++--\n 1 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 7456d8a..0532003 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -848,11 +848,12 @@ int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,\n }\n \n static const char mailinfo_usage[] =\n-\t\"git-mailinfo [-k] [-u | --encoding=<encoding>] msg patch <mail >info\";\n+\t\"git-mailinfo [-k] [-u | --encoding=<encoding>] [-x=<field>] msg patch <mail >info\";\n \n int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n {\n \tconst char *def_charset;\n+\tint top;\n \n \t/* NEEDSWORK: might want to do the optional .git/ directory\n \t * discovery\n@@ -862,6 +863,8 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n \tdef_charset = (git_commit_encoding ? git_commit_encoding : \"utf-8\");\n \tmetainfo_charset = def_charset;\n \n+\tfor (top=0; header[top]; top++){ ; }\n+\n \twhile (1 < argc && argv[1][0] == '-') {\n \t\tif (!strcmp(argv[1], \"-k\"))\n \t\t\tkeep_subject = 1;\n@@ -871,7 +874,10 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n \t\t\tmetainfo_charset = NULL;\n \t\telse if (!prefixcmp(argv[1], \"--encoding=\"))\n \t\t\tmetainfo_charset = argv[1] + 11;\n-\t\telse\n+\t\telse if (!prefixcmp(argv[1], \"-x=\")) {\n+\t\t\theader[top] = xmalloc(256*sizeof(char));\n+\t\t\tstrncpy(header[top++], argv[1]+3, 256);\n+\t\t} else\n \t\t\tusage(mailinfo_usage);\n \t\targc--; argv++;\n \t}\n-- \n1.5.0.2.212.gd52f-dirty\n"},{"id":"36478","messageId":"117321826466-git-send-email-dzickus@redhat.com","threadId":"7118","inReplyTo":"11732182643385-git-send-email-dzickus@redhat.com","subject":"[PATCH 3/5] restrict the patch filtering","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2007-03-06T21:57:41Z","receivedAt":"2007-03-06T21:57:41Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"I have come across many emails that use long strings of '-'s as separators\nfor ideas.  This patch below limits the separator to only 3 '-', with the\nintent that long string of '-'s will stay in the commit msg and not in the\npatch file.\n\nSigned-off-by: Don Zickus <dzickus@redhat.com>\n\n---\nI purposedly separated this patch out because I wasn't sure if anyone would\nhave objections to it.  I tested it on numerous emails with and with patches\nand didn't see any issues.\n---\n builtin-mailinfo.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 0532003..0d7c77b 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -666,7 +666,8 @@ static int handle_commit_msg(char *line)\n \t}\n \n \tif (!memcmp(\"diff -\", line, 6) ||\n-\t    !memcmp(\"---\", line, 3) ||\n+\t    !memcmp(\"---\\n\", line, 4) ||\n+\t    !memcmp(\"--- \", line, 4) ||\n \t    !memcmp(\"Index: \", line, 7)) {\n \t\tfclose(cmitmsg);\n \t\tcmitmsg = NULL;\n-- \n1.5.0.2.212.gd52f-dirty\n"},{"id":"36480","messageId":"11732182641895-git-send-email-dzickus@redhat.com","threadId":"7118","inReplyTo":"117321826466-git-send-email-dzickus@redhat.com","subject":"[PATCH 4/5] Add a couple more test cases to the suite.","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2007-03-06T21:57:42Z","receivedAt":"2007-03-06T21:57:42Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"They handle cases where there is no attached patch.\n\nSigned-off-by: Don Zickus <dzickus@redhat.com>\n---\n t/t5100-mailinfo.sh |    2 +-\n t/t5100/info0007    |    5 +++++\n t/t5100/info0008    |    5 +++++\n t/t5100/msg0007     |    2 ++\n t/t5100/msg0008     |    4 ++++\n t/t5100/sample.mbox |   18 ++++++++++++++++++\n 6 files changed, 35 insertions(+), 1 deletions(-)\n create mode 100644 t/t5100/info0007\n create mode 100644 t/t5100/info0008\n create mode 100644 t/t5100/msg0007\n create mode 100644 t/t5100/msg0008\n create mode 100644 t/t5100/patch0007\n create mode 100644 t/t5100/patch0008\n\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex 4d2b781..ca96918 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \\\n \t'git-mailsplit -o. ../t5100/sample.mbox >last &&\n \tlast=`cat last` &&\n \techo total is $last &&\n-\ttest `cat last` = 6'\n+\ttest `cat last` = 8'\n \n for mail in `echo 00*`\n do\ndiff --git a/t/t5100/info0007 b/t/t5100/info0007\nnew file mode 100644\nindex 0000000..49bb0fe\n--- /dev/null\n+++ b/t/t5100/info0007\n@@ -0,0 +1,5 @@\n+Author: A U Thor\n+Email: a.u.thor@example.com\n+Subject: another patch\n+Date: Fri, 9 Jun 2006 00:44:16 -0700\n+\ndiff --git a/t/t5100/info0008 b/t/t5100/info0008\nnew file mode 100644\nindex 0000000..e8a2951\n--- /dev/null\n+++ b/t/t5100/info0008\n@@ -0,0 +1,5 @@\n+Author: Junio C Hamano\n+Email: junio@kernel.org\n+Subject: another patch\n+Date: Fri, 9 Jun 2006 00:44:16 -0700\n+\ndiff --git a/t/t5100/msg0007 b/t/t5100/msg0007\nnew file mode 100644\nindex 0000000..71b23c0\n--- /dev/null\n+++ b/t/t5100/msg0007\n@@ -0,0 +1,2 @@\n+Here is an empty patch from A U Thor.\n+\ndiff --git a/t/t5100/msg0008 b/t/t5100/msg0008\nnew file mode 100644\nindex 0000000..a80ecb9\n--- /dev/null\n+++ b/t/t5100/msg0008\n@@ -0,0 +1,4 @@\n+>Here is an empty patch from A U Thor.\n+\n+Hey you forgot the patch!\n+\ndiff --git a/t/t5100/patch0007 b/t/t5100/patch0007\nnew file mode 100644\nindex 0000000..e69de29\ndiff --git a/t/t5100/patch0008 b/t/t5100/patch0008\nnew file mode 100644\nindex 0000000..e69de29\ndiff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox\nindex 86bfc27..b80c981 100644\n--- a/t/t5100/sample.mbox\n+++ b/t/t5100/sample.mbox\n@@ -386,3 +386,21 @@ index 9123cdc..918dcf8 100644\n -- \n 1.4.0.g6f2b\n \n+From nobody Mon Sep 17 00:00:00 2001\n+From: A U Thor <a.u.thor@example.com>\n+Date: Fri, 9 Jun 2006 00:44:16 -0700\n+Subject: [PATCH] another patch\n+\n+Here is an empty patch from A U Thor.\n+\n+From nobody Mon Sep 17 00:00:00 2001\n+From: Junio C Hamano <junio@kernel.org>\n+Date: Fri, 9 Jun 2006 00:44:16 -0700\n+Subject: re: [PATCH] another patch\n+\n+From: A U Thor <a.u.thor@example.com>\n+Subject: [PATCH] another patch\n+>Here is an empty patch from A U Thor.\n+\n+Hey you forgot the patch!\n+\n-- \n1.5.0.2.212.gd52f-dirty\n"},{"id":"36479","messageId":"11732182643193-git-send-email-dzickus@redhat.com","threadId":"7118","inReplyTo":"11732182641895-git-send-email-dzickus@redhat.com","subject":"[PATCH 5/5] fix a utf8 issue in t5100/patch005","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2007-03-06T21:57:43Z","receivedAt":"2007-03-06T21:57:43Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"This issue popped up when testing my changes.  I believe the patch is the\nintended output that git-mailinfo should provide.\n\nSigned-off-by: Don Zickus <dzickus@redhat.com>\n---\n t/t5100/patch0005 |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5100/patch0005 b/t/t5100/patch0005\nindex 7d24b24..e7d6f66 100644\n--- a/t/t5100/patch0005\n+++ b/t/t5100/patch0005\n@@ -61,7 +61,7 @@ diff --git a/git-cvsimport-script b/git-cvsimport-script\n  \t\tpush(@old,$fn);\n \n -- \n-David Kågedal\n+David KÃ¥gedal\n -\n To unsubscribe from this list: send the line \"unsubscribe git\" in\n the body of a message to majordomo@vger.kernel.org\n-- \n1.5.0.2.212.gd52f-dirty\n"},{"id":"36482","messageId":"Pine.LNX.4.64.0703061406580.5963@woody.linux-foundation.org","threadId":"7118","inReplyTo":"117321826466-git-send-email-dzickus@redhat.com","subject":"Re: [PATCH 3/5] restrict the patch filtering","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-06T22:19:13Z","receivedAt":"2007-03-06T22:19:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 6 Mar 2007, Don Zickus wrote:\n>\n> I have come across many emails that use long strings of '-'s as separators\n> for ideas.  This patch below limits the separator to only 3 '-', with the\n> intent that long string of '-'s will stay in the commit msg and not in the\n> patch file.\n\nAck. I think this is better than what we have now. My only question is \nwhether we should allow arbitrary whitespace after the \"---\", ie all of \nspace/tab/newline. Your patch just does space/newline (and only _one_ \nspace).\n\nI think the space-only is probably fine - we don't allow tabs there in \ndiffs, and the only reason I mention whitespace is that in an editor it's \nactually really hard to see spaces at the ends of lines, so I think it's \nbad form to behave differently for\n\n\t\"---\\n\"\n\nthan for\n\n\t\"--- \\t \\n\"\n\nwhen they both *look* the same in an editor.\n\n> Signed-off-by: Don Zickus <dzickus@redhat.com>\nAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n\nAlthough I'd be even happier with something like\n\n\tstatic inline int patchbreak(const char *line)\n\t{\n\t\t/* Beginning of a \"diff -\" header? */\n\t\tif (!memcmp(\"diff -\", line, 6))\n\t\t\treturn 1;\n\n\t\t/* CVS \"Index: \" line? */\n\t\tif (!memcmp(\"Index: \", line, 7))\n\t\t\treturn 1;\n\n\t\t/*\n\t\t * \"--- <filename>\" starts patches without headers\n\t\t * \"---<sp>*\" is a manual separator\n\t\t */\n\t\tif (!memcmp(\"---\", line, 3)) {\n\t\t\tline += 3;\n\t\t\t/* space followed by a filename? */\n\t\t\tif (line[0] == ' ' && !isspace(line[1]))\n\t\t\t\treturn 1;\n\t\t\t/* Just whitespace? */\n\t\t\tfor (;;) {\n\t\t\t\tunsigned char c = *line++;\n\t\t\t\tif (c == '\\n')\n\t\t\t\t\treturn 1;\n\t\t\t\tif (!isspace(c))\n\t\t\t\t\tbreak;\n\t\t\t}\n\t\t\treturn 0;\n\t\t}\n\t\treturn 0;\n\t}\n\nwhich is totally untested, of course.\n\n\t\tLinus\n"},{"id":"36530","messageId":"7v8xe9k460.fsf@assigned-by-dhcp.cox.net","threadId":"7118","inReplyTo":"1173218263315-git-send-email-dzickus@redhat.com","subject":"Re: [PATCH 1/5] builtin-mailinfo.c infrastrcture changes","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-07T05:34:31Z","receivedAt":"2007-03-07T05:34:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Don Zickus <dzickus@redhat.com> writes:\n\n> List of major changes/fixes:\n> - can't create empty patch files fix\n> - empty patch files don't fail, this failure will come inside git-am\n> - multipart boundaries are now handled\n> - only output inbody headers if a patch exists otherwise assume those\n> headers are part of the reply and instead output the original headers\n> - decode and filter base64 patches correctly\n> - various other accidental fixes\n>\n> I believe I didn't break any existing functionality or compatibility (other\n> than what I describe above, which is really only the empty patch file).\n>\n> I tested this through various mailing list archives and everything seemed to\n> parse correctly (a couple thousand emails).\n\nThanks.\n\n> @@ -177,17 +175,35 @@ static int slurp_attr(const char *line, const char *name, char *attr)\n>  \treturn 1;\n>  }\n>  \n> -static int handle_subcontent_type(char *line)\n> +struct content_type {\n> +\tchar *boundary;\n> +\tint boundary_len;\n> +};\n> +\n> +static struct content_type content[]={\n> +\t{ NULL },\n> +\t{ NULL },\n> +\t{ NULL },\n> +\t{ NULL },\n> +\t{ NULL },\n> +};\n\nThe reason why this array has 5 elements (not 4, not 6) is...?\n\n> +\n> +static struct content_type *content_top = content;\n> +\n> +static int handle_content_type(char *line)\n>  {\n> +\tchar boundary[256];\n> +\n> +\tif (strcasestr(line, \"text/\") < 0)\n> +\t\t message_type = TYPE_OTHER;\n\nDid you mean\n\n\tif (strncasecmp(line, \"text/\", 5))\n\nIt makes me wonder how a couple thousand mails (or for that\nmatter our own testsuite) could have passed the test with a\nthing like this...\n\n> +\tif (slurp_attr(line, \"boundary=\", boundary + 2)) {\n> +\t\tmemcpy(boundary, \"--\", 2);\n> +\t\tcontent_top++;\n> +\t\tcontent_top->boundary_len = strlen(boundary);\n> +\t\tcontent_top->boundary = xmalloc(content_top->boundary_len+1);\n> +\t\tstrcpy(content_top->boundary, boundary);\n>  \t}\n\nDoes anybody check the content[] stack overflow?\n\n> @@ -341,57 +290,65 @@ static void cleanup_space(char *buf)\n>  }\n>  \n>  static void decode_header(char *it);\n> -typedef int (*header_fn_t)(char *);\n> -struct header_def {\n> -\tconst char *name;\n> -\theader_fn_t func;\n> -\tint namelen;\n> +static char *header[10] = {\n> +\t\"From\",\n> +\t\"Subject\",\n> +\t\"Date\",\n> +\tNULL,\n> +\tNULL,\n> +\tNULL,\n>  };\n\nWhy initialize only three with NULL when you have four more?\nWhat are the 7 entries other than From/Subject/Date for?  Future\nextension?  Wouldn't \n\n\tstatic char *header[] = {\n        \t\"From\", \"Subject\", \"Date\", NULL,\n\t};\n\nor even:\n\n\tstatic char *header[] = {\n        \t\"From\", \"Subject\", \"Date\",\n\t};\n\neasier to handle (for the latter, you would need your loop with:\n\n\tfor (i = 0; i < ARRAY_SIZE(header); i++)\n        \t...\n\n> +\t/* Content stuff */\n> +\tif (!strncasecmp(line, \"Content-Type: \", 14)) {\n\nI'd rather not insist SP after colon (I do not think it even has\nto exist, but I think we check for isspace() in the current code).\n\n> +static int handle_boundary(void)\n> +{\n> +again:\n> +\tif (!memcmp(line+content_top->boundary_len, \"--\", 2)) {\n> +\t\t/* we hit an end boundary */\n> +\t\t/* pop the current boundary off the stack */\n> +\t\tfree(content_top->boundary);\n> +\t\tcontent_top--;\n> +\t\thandle_filter(\"\\n\");\n\nStack underflow?\n\n> +static void handle_body(void)\n>  {\n> ...\n> +\t\tswitch (transfer_encoding) {\n> +\t\tcase TE_BASE64:\n> +\t\t{\n> +\t\t\t/* binary data most likely doesn't have newlines */\n> +\t\t\tif (message_type != TYPE_TEXT) {\n> +\t\t\t\trc=handle_filter(line);\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\n> +\t\t\t/* this is a decoded line that may contain\n> +\t\t\t * multiple new lines.  Pass only one chunk\n> +\t\t\t * at a time to handle_filter()\n> +\t\t\t */\n> +\n> +\t\t\tchar *op=line;\n> +\n\nbuiltin-mailinfo.c:786: warning: ISO C90 forbids mixed declarations and code.\n\n> @@ -809,18 +833,16 @@ int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,\n>  \t\tfclose(cmitmsg);\n>  \t\treturn -1;\n>  \t}\n> +\n> +\tp_hdr_data = xcalloc(10, sizeof(char *));\n> +\ts_hdr_data = xcalloc(10, sizeof(char *));\n\nThese tens look unexplained magic...\n\n> diff --git a/git-am.sh b/git-am.sh\n> index 2c73d11..8fa466a 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -290,6 +290,7 @@ do\n>  \t\tgit-mailinfo $keep $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n>  \t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n>  \t\t\tstop_here $this\n> +\t\ttest -s $dotest/patch || stop_here $this\n>  \t\tgit-stripspace < \"$dotest/msg\" > \"$dotest/msg-clean\"\n>  \t\t;;\n>  \tesac\n\nI think this interface change probably is a good thing.  I\nwonder if we need a matching change to applymbox, though.\n"},{"id":"36558","messageId":"20070307171744.GL11029@redhat.com","threadId":"7118","inReplyTo":"7v8xe9k460.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/5] builtin-mailinfo.c infrastrcture changes","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2007-03-07T17:17:44Z","receivedAt":"2007-03-07T17:17:44Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Tue, Mar 06, 2007 at 09:34:31PM -0800, Junio C Hamano wrote:\n> \n> > @@ -177,17 +175,35 @@ static int slurp_attr(const char *line, const char *name, char *attr)\n> >  \treturn 1;\n> >  }\n> >  \n> > -static int handle_subcontent_type(char *line)\n> > +struct content_type {\n> > +\tchar *boundary;\n> > +\tint boundary_len;\n> > +};\n> > +\n> > +static struct content_type content[]={\n> > +\t{ NULL },\n> > +\t{ NULL },\n> > +\t{ NULL },\n> > +\t{ NULL },\n> > +\t{ NULL },\n> > +};\n> \n> The reason why this array has 5 elements (not 4, not 6) is...?\n\na flip of a coin.  I got lazy and didn't feel like implementing linked\nlists.  So instead I decided to use static char arrays.  I can create a\n#define <arraysize> with a comment on its intended use.  Or do you prefer\na different approach. \n \n> \n> > +\n> > +static struct content_type *content_top = content;\n> > +\n> > +static int handle_content_type(char *line)\n> >  {\n> > +\tchar boundary[256];\n> > +\n> > +\tif (strcasestr(line, \"text/\") < 0)\n> > +\t\t message_type = TYPE_OTHER;\n> \n> Did you mean\n> \n> \tif (strncasecmp(line, \"text/\", 5))\n> \n> It makes me wonder how a couple thousand mails (or for that\n> matter our own testsuite) could have passed the test with a\n> thing like this...\n\nOoops.  Thanks for catching that.  The default TYPE_TEXT works for almost\nall situations, which is why it passes my tests.  The only reason why I\nput that code in there is to handle very large base64 binary blobs (that\ncould overflow the char arrays over 2k bytes).  I didn't have any binary\nblobs that large to test so I never ran into this bug.  I'll fix it up.\n\n> \n> > +\tif (slurp_attr(line, \"boundary=\", boundary + 2)) {\n> > +\t\tmemcpy(boundary, \"--\", 2);\n> > +\t\tcontent_top++;\n> > +\t\tcontent_top->boundary_len = strlen(boundary);\n> > +\t\tcontent_top->boundary = xmalloc(content_top->boundary_len+1);\n> > +\t\tstrcpy(content_top->boundary, boundary);\n> >  \t}\n> \n> Does anybody check the content[] stack overflow?\n\nnope.  meant too though.  I'll fix that too.\n\n> \n> > @@ -341,57 +290,65 @@ static void cleanup_space(char *buf)\n> >  }\n> >  \n> >  static void decode_header(char *it);\n> > -typedef int (*header_fn_t)(char *);\n> > -struct header_def {\n> > -\tconst char *name;\n> > -\theader_fn_t func;\n> > -\tint namelen;\n> > +static char *header[10] = {\n> > +\t\"From\",\n> > +\t\"Subject\",\n> > +\t\"Date\",\n> > +\tNULL,\n> > +\tNULL,\n> > +\tNULL,\n> >  };\n> \n> Why initialize only three with NULL when you have four more?\n> What are the 7 entries other than From/Subject/Date for?  Future\n> extension?  Wouldn't \n> \n> \tstatic char *header[] = {\n>         \t\"From\", \"Subject\", \"Date\", NULL,\n> \t};\n> \n> or even:\n> \n> \tstatic char *header[] = {\n>         \t\"From\", \"Subject\", \"Date\",\n> \t};\n> \n> easier to handle (for the latter, you would need your loop with:\n> \n> \tfor (i = 0; i < ARRAY_SIZE(header); i++)\n>         \t...\n> \n\nsee patch 2/5 for a better idea of the direction I was going with this.\nAgain another lazy trick to avoid linked lists and instead use static char\narrays to easily handle new content.  Probably wouldn't hurt for me to clean\nthis up too.\n\n\n> > +\t/* Content stuff */\n> > +\tif (!strncasecmp(line, \"Content-Type: \", 14)) {\n> \n> I'd rather not insist SP after colon (I do not think it even has\n> to exist, but I think we check for isspace() in the current code).\n\nok.\n\n> \n> > +static int handle_boundary(void)\n> > +{\n> > +again:\n> > +\tif (!memcmp(line+content_top->boundary_len, \"--\", 2)) {\n> > +\t\t/* we hit an end boundary */\n> > +\t\t/* pop the current boundary off the stack */\n> > +\t\tfree(content_top->boundary);\n> > +\t\tcontent_top--;\n> > +\t\thandle_filter(\"\\n\");\n> \n> Stack underflow?\n\nyup.  i'll fix this.\n\n> \n> > +static void handle_body(void)\n> >  {\n> > ...\n> > +\t\tswitch (transfer_encoding) {\n> > +\t\tcase TE_BASE64:\n> > +\t\t{\n> > +\t\t\t/* binary data most likely doesn't have newlines */\n> > +\t\t\tif (message_type != TYPE_TEXT) {\n> > +\t\t\t\trc=handle_filter(line);\n> > +\t\t\t\tbreak;\n> > +\t\t\t}\n> > +\n> > +\t\t\t/* this is a decoded line that may contain\n> > +\t\t\t * multiple new lines.  Pass only one chunk\n> > +\t\t\t * at a time to handle_filter()\n> > +\t\t\t */\n> > +\n> > +\t\t\tchar *op=line;\n> > +\n> \n> builtin-mailinfo.c:786: warning: ISO C90 forbids mixed declarations and code.\n\nhmm. don't know why gcc 4.1.1 didn't catch that.  anyway, the binary data\npart was added much later.  should be easy to fix.\n\n> \n> > @@ -809,18 +833,16 @@ int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,\n> >  \t\tfclose(cmitmsg);\n> >  \t\treturn -1;\n> >  \t}\n> > +\n> > +\tp_hdr_data = xcalloc(10, sizeof(char *));\n> > +\ts_hdr_data = xcalloc(10, sizeof(char *));\n> \n> These tens look unexplained magic...\n\ni'll create a #define for those too.\n\n> \n> > diff --git a/git-am.sh b/git-am.sh\n> > index 2c73d11..8fa466a 100755\n> > --- a/git-am.sh\n> > +++ b/git-am.sh\n> > @@ -290,6 +290,7 @@ do\n> >  \t\tgit-mailinfo $keep $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n> >  \t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n> >  \t\t\tstop_here $this\n> > +\t\ttest -s $dotest/patch || stop_here $this\n> >  \t\tgit-stripspace < \"$dotest/msg\" > \"$dotest/msg-clean\"\n> >  \t\t;;\n> >  \tesac\n> \n> I think this interface change probably is a good thing.  I\n> wonder if we need a matching change to applymbox, though.\n\nthat and git-quiltimport too.\n\nI'll send out another draft this afternoon.\n\nCheers,\nDon\n"}]}