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

On 01/02/2019 10:16, Corinna Vinschen wrote:
> 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.

Ok, I checked everything in modulo the register removal part.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : [email protected]
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.