From: Junio C Hamano <junkio@cox.net>
Date: Thu, 24 May 2007 19:47:00 GMT
Subject: Re: [PATCH] Teach mailsplit about Maildir's
Message-ID: <7vd50qvwaz.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070521182052.GB4696@ferdyx.org>

"Fernando J. Pereda" <ferdy@gentoo.org> writes:

> Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
> ---
>  Documentation/git-am.txt        |    8 ++-
>  Documentation/git-mailsplit.txt |   13 +++-
>  builtin-mailsplit.c             |  139 ++++++++++++++++++++++++++++++++-------
>  builtin.h                       |    2 +-
>  4 files changed, 132 insertions(+), 30 deletions(-)
>
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index ba79773..25cf84a 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -12,7 +12,8 @@ SYNOPSIS
>  'git-am' [--signoff] [--dotest=<dir>] [--keep] [--utf8 | --no-utf8]
>           [--3way] [--interactive] [--binary]
>           [--whitespace=<option>] [-C<n>] [-p<n>]
> -         <mbox>...
> +         <mbox>|<Maildir>...
> +
>  'git-am' [--skip | --resolved]

Does the document still format Ok if you add a blank line there
(not a rhetorical question -- I haven't checked)?

> diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
> index 3bca855..454f943 100644
> --- a/builtin-mailsplit.c
> +++ b/builtin-mailsplit.c
> @@ -6,9 +6,10 @@
>   */
>  #include "cache.h"
>  #include "builtin.h"
> +#include "path-list.h"
>  
>  static const char git_mailsplit_usage[] =
> -"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>...";
> +"git-mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> <mbox>|<Maildir>...";
>  
>  static int is_from_line(const char *line, int len)
>  {
> @@ -96,44 +97,106 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
>  	exit(1);
>  }
>  
> -int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip)
> +static int populate_maildir_list(struct path_list *list, const char *path)
>  {
> -	char *name = xmalloc(strlen(dir) + 2 + 3 * sizeof(skip));
> +	DIR *dir;
> +	struct dirent *dent;
> +
> +	if ((dir = opendir(path)) == NULL) {
> +		error("cannot diropen %s (%s)", path, strerror(errno));
> +		return -1;
> +	}

Didn't you just fail opendir, not diropen?

> +
> +	while ((dent = readdir(dir)) != NULL) {
> +		if (dent->d_name[0] == '.')
> +			continue;
> +		path_list_insert(dent->d_name, list);
> +	}
> +
> +	closedir(dir);
> +
> +	return 1;
> +}

Usually we signal success by returning 0.

> +static int split_maildir(const char *maildir, const char *dir,
> +	int nr_prec, int skip)
> +{
> +	char file[PATH_MAX];
> +	char curdir[PATH_MAX];
> +	char name[PATH_MAX];
>  	int ret = -1;
> +	struct path_list list = {NULL, 0, 0, 1};
>  
> +	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
> +	if (populate_maildir_list(&list, curdir) < 0)
> +		goto out;
>  
> +	int i;

Decl-after-statement.

> +	for (i = 0; i < list.nr; i++) {
> +		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
> +		FILE *f = fopen(file, "r");

Likewise.

> @@ -186,9 +249,39 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
> ...
> +		if (ret < 0) {
> +			error("cannot split patches from %s", arg);
> +			return 1;
> +		}
> +		num += ret;
> +	}
> +
> +	printf("%d\n", num);
>  
> -	return ret == -1;
> +	return 0;
>  }

We do not signal error anymore from the command?