Re: [PATCH] Teach mailsplit about Maildir's

["Fernando J. Pereda" <ferdy@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On Fri, Apr 27, 2007 at 01:54:55AM -0700, Junio C Hamano wrote:
> "Fernando J. Pereda" <ferdy@gentoo.org> writes:
> 
> > +int split_maildir(const char *maildir, const char *dir, int nr_prec, int skip)
> >  {
> > ...
> > +	while ((maildent = readdir(mddir)) != NULL) {
> > +		FILE *f;
> > +
> > +		snprintf(file, sizeof(file), "%s/%s",
> > +				curdir, maildent->d_name);
> > +
> > +		if (maildent->d_name[0] == '.')
> > +			continue;
> >  ...
> > +		sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
> > +		split_one(f, name, 1);
> > +
> > +		fclose(f);
> > +	}
> > +
> > +	closedir(mddir);
> > +
> > +	ret = skip;
> > +out:
> > +	return ret;
> > +}
> 
> I do not personally deal with maildir so I do not know for sure,
> but this feels very wrong.
> 
> What order are you emitting the output?
> 
> split_mbox() is designed to number the messages the same order
> as they are found in the mailbox, but the above loop relies on
> readdir() to give them in a reasonable order to you, which does
> not seem a right assumption to me (otherwise "/bin/ls" and
> friends would not sort what they read from the filesystem would
> they?).

It is indeed very wrong. You can't sort them without opening and parsing
the headers. Please drop this patch.

Sorry for the noise.

- ferdy

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]