Re: [RFA/libiberty] Enhance FILENAME_CMP for Windows filesystems

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 28 Mar 2007 13:24:43 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> 
> Here is a revised version of the patch that should address both
> comments (copyright notice, and lack of documentation):

Sorry for chiming in only now, but I have a few minor comments:

> @deftypefn Extension int filename_cmp (const char *@var{s1}, const char *@var{s2})
> 
> Return zero if the two paths @var{s1} and @var{s2} are equivalent.

The GNU project frowns on using `path' to mean a file name.  `Path' is
reserved to $PATH-like lists of directories.  In this context, I
suggest to use `file name' instead.  (There's one more instance of
using `path' in the documentation of this function.)

> If not equivalent, the returned value is similar to what strcmp would

"strcmp" should be in @code{}, as it is a C symbol.

> This function does not normalize path names. As a result, this function
                                              ^
Two blanks, please.

> int
> filename_cmp (const char *s1, const char *s2)
> {
> #ifndef HAVE_DOS_BASED_FILE_SYSTEM
>   return strcmp(s1, s2);

While I realize that the original FILENAME_CMP macro did the same, as
long as we are trying to do better, wouldn't it be nice if this
function also collapsed multiple consecutive slashes or backslashes?

>   for (;;)
>     {
>       int c1 = tolower (*s1);
>       int c2 = tolower (*s2);

Are we sure that strncasecmp's behavior wrt to locales is identical to
that of tolower's?  If not, the above will introduce a bug in
non-English locales.