Re: [PATCH 7/8] a) Use strcoll() in opendir() and alphasort()

Corinna Vinschen <[email protected]> Fri, 1 Feb 2019 10:16:17 +0100
Newsgroups	gmane.comp.lib.newlib
Message-ID	<[email protected]>
On Feb  1 07:55, Sebastian Huber wrote:
> On 31/01/2019 14:05, Sebastian Huber wrote:
> > From: ache <[email protected]>
> > 
> > as POSIX 2008 requires. It also matches now how our 'ls' works for years.
> > 
> > b) Remove comment expressed 2 fears:
> >   1) One just simple describe how strcoll() works in _any_ context,
> >   not for directories only. Are we plan to remove strcoll() from everything
> >   just because it is little more complex than strcmp()? I doubt, and
> >   directories give nothing different here. Moreover, strcoll() used
> >   in 'ls' for years and nobody complaints yet.
> > 
> >   2) Plain wrong statement about undefined strcoll() behaviour. strcoll()
> >   always gives predictable results, falling back to strcmp() on any
> >   trouble, see strcoll(3).
> > 
> > No objections from -current list discussion.
> > ---
> >   newlib/libc/posix/scandir.c | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/newlib/libc/posix/scandir.c b/newlib/libc/posix/scandir.c
> > index 94c583761..13354c05e 100644
> > --- a/newlib/libc/posix/scandir.c
> > +++ b/newlib/libc/posix/scandir.c
> > @@ -142,12 +142,13 @@ fail:
> >   /*
> >    * Alphabetic order comparison routine for those who want it.
> > + * POSIX 2008 requires that alphasort() uses strcoll().
> >    */
> >   int
> > -alphasort (const struct dirent **d1,
> > -       const struct dirent **d2)
> > +alphasort(const struct dirent **d1, const struct dirent **d2)
> >   {
> > -       return(strcmp((*d1)->d_name, (*d2)->d_name));
> > +
> > +	return (strcoll((*d1)->d_name, (*d2)->d_name));
> >   }
> >   #endif /* ! HAVE_OPENDIR */
> 
> After looking into newlib/libc/string/strcoll.c, this patch makes no sense.
> I will not apply it.

I disagree.  POSIX requires alpphasort to call strcoll, so this is
clearly a bug in newlib's implementation.  Also, what if, at one point,
somebody improves newlib's strcoll?

Cygwin's alphasort uses strcoll as well.  I'm just not quite sure
why Cygwin still has its own implementation.  There's nothing
Cygwin-specific in there, except for the strcmp/strcoll difference.
I guess I'll remove it after the Cygwin 3.0 release.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
signature.asc (application/pgp-signature, 833 B)