Re: [bug report] git-am applying maildir patches in reverse
- From
Jeff King <peff@peff.net>
- Date
- Mar 2, 2013, 00:41 UTC
- Message-ID
- <20130302004120.GB14936@sigill.intra.peff.net>
- In-Reply-To
- <7v1ubyek9n.fsf@alter.siamese.dyndns.org>
On Fri, Mar 01, 2013 at 04:08:04PM -0800, Junio C Hamano wrote:
Show 8 quoted lines
> > +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