On Fri, Mar 01, 2013 at 04:08:04PM -0800, Junio C Hamano wrote:
> > +static int maildir_filename_cmp(const char *a, const char *b)
> > +{
> > + while (1) {
>
> It is somewhat funny that we do not need to check !*a or !*b in this
> loop. As long as readdir() does not return duplicates, we won't be
> comparing the same strings with this function, and we won't read
> past '\0' at the end of both a and b at the same time.
Ugh, thanks for catching. I had structured this differently at first
with a while(1), and then after refactoring it forgot to update the loop
condition. Even if it is fine without duplicates, we should not rely on
that not to cause an infinite loop (in case the function is ever used
again). But moreover, you can trigger the problem even now, since we
would parse "foo_0123" and "foo_123" as equivalent, and hit the NUL.
I think this needs squashed in:
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 772c668..1ddbff4 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -132,7 +132,7 @@ static int maildir_filename_cmp(const char *a, const char *b)
static int maildir_filename_cmp(const char *a, const char *b)
{
- while (1) {
+ while (*a && *b) {
if (isdigit(*a) && isdigit(*b)) {
long int na, nb;
na = strtol(a, (char **)&a, 10);
@@ -148,6 +148,7 @@ static int maildir_filename_cmp(const char *a, const char *b)
b++;
}
}
+ return *a - *b;
}
static int split_maildir(const char *maildir, const char *dir,
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html