Re: [PATCH] Teach mailsplit about Maildir's

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Teach mailsplit about Maildir's

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:07

"Fernando J. Pereda" [off-list ref] 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?).

Re: [PATCH] Teach mailsplit about Maildir's

From: Fernando J. Pereda <hidden>
Date: 2016-06-15 22:43:07

On Fri, Apr 27, 2007 at 01:54:55AM -0700, Junio C Hamano wrote:
"Fernando J. Pereda" [off-list ref] writes:
quoted
+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help