{"thread":{"id":"7855","subject":"[PATCH] Teach mailsplit about Maildir's","startedAt":"2007-04-26T19:24:39Z","lastAt":"2007-04-27T08:59:51Z","messageCount":4,"participants":["Fernando J. Pereda","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"40520","messageId":"20070426192439.GA6976@ferdyx.org","threadId":"7855","inReplyTo":null,"subject":"[PATCH] Teach mailsplit about Maildir's","fromName":"Fernando J. Pereda","fromEmail":"ferdy@gentoo.org","sentAt":"2007-04-26T19:24:39Z","receivedAt":"2007-04-26T19:24:39Z","isPatch":true,"sender":{"key":"ferdy@gentoo.org","avatar":null},"body":"Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>\n---\n\n builtin-mailsplit.c |  107 ++++++++++++++++++++++++++++++++++++++++++---------\n builtin.h           |    2 +-\n 2 files changed, 89 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex 3bca855..e0a283d 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -96,44 +96,93 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)\n \texit(1);\n }\n \n-int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)\n+int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)\n {\n-\tchar *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));\n+\tchar file[PATH_MAX];\n+\tchar curdir[PATH_MAX];\n+\tchar name[PATH_MAX];\n+\tDIR *mddir;\n+\tstruct dirent *maildent;\n \tint ret = -1;\n \n-\twhile (*mbox) {\n-\t\tconst char *file = *mbox++;\n-\t\tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n-\t\tint file_done = 0;\n+\tsnprintf(curdir, sizeof(curdir), \"%s/cur\", maildir);\n+\tif ((mddir = opendir(curdir)) == NULL) {\n+\t\terror(\"cannot diropen %s (%s)\", curdir, strerror(errno));\n+\t\tgoto out;\n+\t}\n+\n+\twhile ((maildent = readdir(mddir)) != NULL) {\n+\t\tFILE *f;\n+\n+\t\tsnprintf(file, sizeof(file), \"%s/%s\",\n+\t\t\t\tcurdir, maildent->d_name);\n+\n+\t\tif (maildent->d_name[0] == '.')\n+\t\t\tcontinue;\n \n-\t\tif ( !f ) {\n-\t\t\terror(\"cannot open mbox %s\", file);\n+\t\tf = fopen(file, \"r\");\n+\t\tif (!f) {\n+\t\t\terror(\"cannot open mail %s (%s)\", file, strerror(errno));\n \t\t\tgoto out;\n \t\t}\n \n \t\tif (fgets(buf, sizeof(buf), f) == NULL) {\n-\t\t\tif (f == stdin)\n-\t\t\t\tbreak; /* empty stdin is OK */\n-\t\t\terror(\"cannot read mbox %s\", file);\n+\t\t\terror(\"cannot read mail %s (%s)\", file, strerror(errno));\n \t\t\tgoto out;\n \t\t}\n \n-\t\twhile (!file_done) {\n-\t\t\tsprintf(name, \"%s/%0*d\", dir, nr_prec, ++skip);\n-\t\t\tfile_done = split_one(f, name, allow_bare);\n+\t\tsprintf(name, \"%s/%0*d\", dir, nr_prec, ++skip);\n+\t\tsplit_one(f, name, 1);\n+\n+\t\tfclose(f);\n+\t}\n+\n+\tclosedir(mddir);\n+\n+\tret = skip;\n+out:\n+\treturn ret;\n+}\n+\n+int split_mbox(const char *file, const char *dir, int allow_bare,\n+\t\tint nr_prec, int skip)\n+{\n+\tchar name[PATH_MAX];\n+\tint ret = -1;\n+\n+\tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n+\tint file_done = 0;\n+\n+\tif (!f) {\n+\t\terror(\"cannot open mbox %s\", file);\n+\t\tgoto out;\n+\t}\n+\n+\tif (fgets(buf, sizeof(buf), f) == NULL) {\n+\t\t/* empty stdin is OK */\n+\t\tif (f != stdin) {\n+\t\t\terror(\"cannot read mbox %s\", file);\n+\t\t\tgoto out;\n \t\t}\n+\t\tfile_done = 1;\n+\t}\n \n-\t\tif (f != stdin)\n-\t\t\tfclose(f);\n+\twhile (!file_done) {\n+\t\tsprintf(name, \"%s/%0*d\", dir, nr_prec, ++skip);\n+\t\tfile_done = split_one(f, name, allow_bare);\n \t}\n+\n+\tif (f != stdin)\n+\t\tfclose(f);\n+\n \tret = skip;\n out:\n-\tfree(name);\n \treturn ret;\n }\n+\n int cmd_mailsplit(int argc, const char **argv, const char *prefix)\n {\n-\tint nr = 0, nr_prec = 4, ret;\n+\tint nr = 0, nr_prec = 4, ret = 0;\n \tint allow_bare = 0;\n \tconst char *dir = NULL;\n \tconst char **argp;\n@@ -186,7 +235,27 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)\n \t\t\targp = stdin_only;\n \t}\n \n-\tret = split_mbox(argp, dir, allow_bare, nr_prec, nr);\n+\twhile (*argp) {\n+\t\tconst char *arg = *argp++;\n+\t\tstruct stat argstat;\n+\n+\t\tif (arg[0] == '-' && arg[1] == 0) {\n+\t\t\tret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (stat(arg, &argstat) == -1) {\n+\t\t\terror(\"cannot stat %s (%s)\", arg, strerror(errno));\n+\t\t\treturn 1;\n+\t\t}\n+\n+\t\tif (S_ISDIR(argstat.st_mode)) {\n+\t\t\tret |= split_maildir(arg, dir, nr_prec, nr);\n+\t\t} else {\n+\t\t\tret |= split_mbox(arg, dir, allow_bare, nr_prec, nr);\n+\t\t}\n+\t}\n+\n \tif (ret != -1)\n \t\tprintf(\"%d\\n\", ret);\n \ndiff --git a/builtin.h b/builtin.h\nindex d3f3a74..39290d1 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -8,7 +8,7 @@ extern const char git_usage_string[];\n \n extern void help_unknown_cmd(const char *cmd);\n extern int mailinfo(FILE *in, FILE *out, int ks, const char *encoding, const char *msg, const char *patch);\n-extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip);\n+extern int split_mbox(const char *file, const char *dir, int allow_bare, int nr_prec, int skip);\n extern void stripspace(FILE *in, FILE *out);\n extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);\n extern void prune_packed_objects(int);\n-- \n1.5.1.2\n"},{"id":"40578","messageId":"20070427083007.GA4690@ferdyx.org","threadId":"7855","inReplyTo":"20070426192439.GA6976@ferdyx.org","subject":"Re: [PATCH] Teach mailsplit about Maildir's","fromName":"Fernando J. Pereda","fromEmail":"ferdy@ferdyx.org","sentAt":"2007-04-27T08:30:07Z","receivedAt":"2007-04-27T08:30:07Z","isPatch":true,"sender":{"key":"ferdy@ferdyx.org","avatar":"https://gravatar.com/avatar/96bf7c1ddf7ccd430255bd12d9d42b212dbc033b28c668a2bdf9c3995aa81e61?d=mp&s=160"},"body":"On Thu, Apr 26, 2007 at 09:24:39PM +0200, Fernando J. Pereda wrote:\n> Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>\n> ---\n> \n>  builtin-mailsplit.c |  107 ++++++++++++++++++++++++++++++++++++++++++---------\n>  builtin.h           |    2 +-\n>  2 files changed, 89 insertions(+), 20 deletions(-)\n>\n\nActually, I forgot to update the documentation, I'll send an updated\npatch.\n\n- ferdy\n\n-- \nFernando J. Pereda Garcimartín\n20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4\n"},{"id":"40581","messageId":"7vd51qp57k.fsf@assigned-by-dhcp.cox.net","threadId":"7855","inReplyTo":"20070426192439.GA6976@ferdyx.org","subject":"Re: [PATCH] Teach mailsplit about Maildir's","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-27T08:54:55Z","receivedAt":"2007-04-27T08:54:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Fernando J. Pereda\" <ferdy@gentoo.org> writes:\n\n> +int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)\n>  {\n> ...\n> +\twhile ((maildent = readdir(mddir)) != NULL) {\n> +\t\tFILE *f;\n> +\n> +\t\tsnprintf(file, sizeof(file), \"%s/%s\",\n> +\t\t\t\tcurdir, maildent->d_name);\n> +\n> +\t\tif (maildent->d_name[0] == '.')\n> +\t\t\tcontinue;\n>  ...\n> +\t\tsprintf(name, \"%s/%0*d\", dir, nr_prec, ++skip);\n> +\t\tsplit_one(f, name, 1);\n> +\n> +\t\tfclose(f);\n> +\t}\n> +\n> +\tclosedir(mddir);\n> +\n> +\tret = skip;\n> +out:\n> +\treturn ret;\n> +}\n\nI do not personally deal with maildir so I do not know for sure,\nbut this feels very wrong.\n\nWhat order are you emitting the output?\n\nsplit_mbox() is designed to number the messages the same order\nas they are found in the mailbox, but the above loop relies on\nreaddir() to give them in a reasonable order to you, which does\nnot seem a right assumption to me (otherwise \"/bin/ls\" and\nfriends would not sort what they read from the filesystem would\nthey?).\n"},{"id":"40586","messageId":"20070427085951.GC4690@ferdyx.org","threadId":"7855","inReplyTo":"7vd51qp57k.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach mailsplit about Maildir's","fromName":"Fernando J. Pereda","fromEmail":"ferdy@gentoo.org","sentAt":"2007-04-27T08:59:51Z","receivedAt":"2007-04-27T08:59:51Z","isPatch":true,"sender":{"key":"ferdy@gentoo.org","avatar":null},"body":"On Fri, Apr 27, 2007 at 01:54:55AM -0700, Junio C Hamano wrote:\n> \"Fernando J. Pereda\" <ferdy@gentoo.org> writes:\n> \n> > +int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)\n> >  {\n> > ...\n> > +\twhile ((maildent = readdir(mddir)) != NULL) {\n> > +\t\tFILE *f;\n> > +\n> > +\t\tsnprintf(file, sizeof(file), \"%s/%s\",\n> > +\t\t\t\tcurdir, maildent->d_name);\n> > +\n> > +\t\tif (maildent->d_name[0] == '.')\n> > +\t\t\tcontinue;\n> >  ...\n> > +\t\tsprintf(name, \"%s/%0*d\", dir, nr_prec, ++skip);\n> > +\t\tsplit_one(f, name, 1);\n> > +\n> > +\t\tfclose(f);\n> > +\t}\n> > +\n> > +\tclosedir(mddir);\n> > +\n> > +\tret = skip;\n> > +out:\n> > +\treturn ret;\n> > +}\n> \n> I do not personally deal with maildir so I do not know for sure,\n> but this feels very wrong.\n> \n> What order are you emitting the output?\n> \n> split_mbox() is designed to number the messages the same order\n> as they are found in the mailbox, but the above loop relies on\n> readdir() to give them in a reasonable order to you, which does\n> not seem a right assumption to me (otherwise \"/bin/ls\" and\n> friends would not sort what they read from the filesystem would\n> they?).\n\nIt is indeed very wrong. You can't sort them without opening and parsing\nthe headers. Please drop this patch.\n\nSorry for the noise.\n\n- ferdy\n\n-- \nFernando J. Pereda Garcimartín\n20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4\n"}]}