From: Fernando J. Pereda <ferdy@gentoo.org>
Date: Thu, 24 May 2007 19:56:08 GMT
Subject: Re: [PATCH] Teach mailsplit about Maildir's
Message-ID: <20070524195608.GA4714@ferdyx.org>
In-Reply-To: <7vd50qvwaz.fsf@assigned-by-dhcp.cox.net>

On Thu, May 24, 2007 at 12:47:00PM -0700, Junio C Hamano wrote:
> "Fernando J. Pereda" <ferdy@gentoo.org> writes:
> >           [--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)?

I think I tried when I first sent it, will try.

> > +
> > +	if ((dir = opendir(path)) == NULL) {
> > +		error("cannot diropen %s (%s)", path, strerror(errno));
> > +		return -1;
> > +	}
> 
> Didn't you just fail opendir, not diropen?

Ouch, will fix

> > +	return 1;
> > +}
> 
> Usually we signal success by returning 0.

Ok. Fair enough.

> 
> > +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.

I did it because other parts of the file do it too. Do you want me to
fix the rest of them?

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

Again, because we were already doing it in other parts of the file. Will
fix.

> 
> > @@ -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?

If we reached that part of the code, there is no error to signal about.
We signal error when either split_mbox or split_maildir fail by
returning 1.

Will fix those issues and send a new patch. Thanks for reviewing it.

- ferdy

-- 
Fernando J. Pereda Garcimartín
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4