Glob sorting doesn't work in some cases

[Dima Kogan <dima@xxxxxxxxxxxxxxx>]

Hi.

I hit a corner case in zsh glob sorting today. Look:

  dima@scrawny:/tmp$ mkdir dir1; touch dir1/file

  dima@scrawny:/tmp$ mkdir dir2; touch dir2/file

  dima@scrawny:/tmp$ mkdir dir3; touch dir3/file

  dima@scrawny:/tmp$ echo dir*/file(om)
  dir2/file dir3/file dir1/file

  dima@scrawny:/tmp$ echo dir*/file(Om)
  dir2/file dir3/file dir1/file

I.e. I made some dummy files, then asked zsh to tell me about them in
most-recently-modified order, and then again in the REVERSE order of
that. Note that neither of these is right, and the reverse part didn't
work either.

This looks like a corner case. If I look at dir*/file(.om) or
dir*/file*(om) then it works fine.

In any case, we can see that when this fails zsh isn't even looking at
the time stamps at all:

  dima@scrawny:/tmp$ strace -e lstat zsh -f -c 'echo dir*/file(om) > /dev/null'
  +++ exited with 0 +++

  dima@scrawny:/tmp$ strace -e lstat zsh -f -c 'echo dir*/file(.om) > /dev/null'
  lstat("dir1/file", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
  lstat("dir3/file", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
  lstat("dir2/file", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
  +++ exited with 0 +++

This can be fixed with this patch:

--- glob.c     2017-11-19 13:25:19.142071142 -0800
+++ glob.c     2017-08-27 13:18:25.000000000 -0700
@@ -387,7 +387,7 @@
            qn = qn->next;
        }
     } else if (!checked) {
-       if (statfullpath(s, NULL, 1)) {
+       if (statfullpath(s, &buf, 1)) {
            unqueue_signals();
            return;
        }

This makes it work. To be clear, I don't completely understand why this
NULL is here, but it was there for 17 years, so there could easily be
something here I don't understand. The patch makes sense, however. This
function has a 'struct stat buf' to receive results of a stat() and an
'int statted' to indicate that we have done this. Every time we actually
do a stat() (via a statfullpath()), we update statted appropriately.
EXCEPT for the path patched above. So when we hit the failure we

1. statfullpath(NULL); this does NOT update buf, but does set statted=1
2. Then a bit later we test if (statted & 1)
3. Which is true. So we then do stuff with buf, which is actually
   uninitialized

Please Cc me when replying. I'm not subscribed to the list.

Thanks!

dima