svn commit: r202572 - head/lib/libc/gen

Bruce Evans brde at optusnet.com.au
Wed Jan 20 08:43:35 UTC 2010


On Wed, 20 Jan 2010, Andrey Chernov wrote:

> On Wed, Jan 20, 2010 at 01:42:08AM +1100, Bruce Evans wrote:
>> The comment was correct.  It says that POSIX requires strcoll() for
>> alphasort(), not for opendir().  Since opendir() is not alphasort(),
>> and it wants plain ASCII sorting to support union file systems, it
>> intentionally doesn't use either alphasort() or strcoll().
>
> Yes, the comment _alone_ was correct, but its place - isn't. Along with
> function name containing _alphasort part it makes impression that
> opendir() uses this type of sort too.

No, it is a comment about opendir()'s comparison function.  It has nothing
to do with scandir(), and the only thing that it has to do with alphasort()
is that it must be different for the reasons described.

> BTW, we already have the same correct comment but in the proper place in
> the scandir.c

That one is quite different.  It describes why alphasort() (now) uses
strcoll().  It is because POSIX says so.  This comment is relatively
useless.  It obviously uses strcoll(), and it is a POSIX interface so
this would be surprising only if it conflicted with POSIX.  The comment
is there mainly for historical reasons, and history belongs in the man
page more than here.  BTW, I don't remember any man page updates for
this.  The man page still only says that alphasort() can be used to
give alphabetical sorting in scandir().

>> Was correct, but it could have been clearer by saying ", so opendir()
>> uses this comparison function instead of alphasort()".
>
> "So", what? The two mentioned things are unrelated and can't be
> concatenated by "so".

They are related.

>> I forget what the old name was.  Having alphasort in the name here was
>> wrong 3 layers deep, since this is not alphasort(), and alphasort() is not
>> an alpha sorting function -- it is a lexicographically-on-the-whole-
>> character-set comparison function.
>
> Yes.
>
>> Correct modulo the name.
>
> What name you suggest, opendir_compar()?

OK.

>> New bug in a comment in scandir(): now has an extra blank line, due to
>> partial removal.
>
> Ok, will remove it a bit later.

I can't see this now (some illusion from my mailer or $TERMCAP
misformatting the patch?), but now I see an extra "the" in it:

     "requires the alphasort() to use strcoll()"

should be either

     "requires that alphasort() uses strcoll()"

(preferred) or

     "requires alphasort() to use strcoll()"

(probably intended, but not too passive).  I thought that you removed
this line completely.  The previous line is even less useful.

Bruce


More information about the svn-src-head mailing list