Re: [hackers] [sbase] [PATCH] ls: Fix sorting of named entries

From: Michael Forney <mforney_AT_mforney.org>
Date: Sun, 1 Nov 2015 04:07:39 -0800

On Sun, Nov 01, 2015 at 03:28:48PM +0100, Quentin Rameau wrote:
> Hi Michael
>
> > Previously, entcmp was being passed struct entry **, when it expected
> > struct entry *.
>
> No it's not, it is being passed a struct entry * by qsort() and expects a const void *.
>
> > Many autoconf-generated configure scripts use `ls -t` to determine whether or
> > not the system clock is behaving correctly. If they are sorted in the wrong
> > order, it produces an error.
>
> Thanks for reporting this issue, I've “fixed” that.
>
> > ls.c | 22 ++++++++++------------
> > 1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/ls.c b/ls.c
> > index c998fc7..c942930 100644
>
> Obviously you didn't test your patch and just managed to break ls.
> It doesn't even fix the issue you brought up.
> The patch will be reverted and ls corrected.

Jeez, I don't think my patch warranted such a hostile response. Can't we
all be nice to each other?

No, I did not think "all other ls implementations are broken as I don't
get the same results gnu ls gives me" as you suggested on IRC. I always
use POSIX as a reference.

Of course I tested my patch. However, my config.mk sets -O2, which must
have caused the uninitialized pointer dereference to succeed. I will
make sure to test with valgrind in the future.

Thanks for your other patch about timestamp resolution. I noticed that
even after this fix, I'd still get the error sporadically, which I
assumed was caused by this.

-Michael
Received on Sun Nov 01 2015 - 13:07:39 CET