[RFA/libiberty] Enhance FILENAME_CMP for Windows filesystems

[RFA/libiberty] Enhance FILENAME_CMP for Windows filesystems

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

Hello,

We are using the FILENAME_CMP macro in GDB quite a bit, and it is
currently defined in include/filenames.h as:

    #define FILENAME_CMP(s1, s2)    strcasecmp(s1, s2)

This is for "DOSish" filesystems. For other filesystems, it's defined as
a straight strcmp:

    #define FILENAME_CMP(s1, s2)    strcmp(s1, s2)

I came across a case where the DWARF debug info unfortunately used '\'
as the directory separator in one case (in the .debug_info section),
while using '/' in another case (in the .debug_line section).

As a result, FILENAME_CMP returned that the following two files were
not equal:

        c:\cygwin\home\brobecke\tgdb\ex\\foo.adb
        c:/cygwin/home/brobecke/tgdb/ex//foo.adb

This causes the following problem in GDB, when we compile the unit
by giving GCC the full path name to the associated file, such as:

        % gcc -c -g c:\full\path\to\foo.adb

The GDB error looks like this:

        % gdb foo
        (gdb) b foo.adb:3
        No line 3 in file "c:\cygwin\home\brobecke\tgdb\ex\\foo.adb".


I propose to add a new filename_cmp function to libiberty:
  - On Unix, no change, it calls strcmp
  - On Windows, we make slash and backslash equal.

This is a modest improvement, since there are many other things we
can do to enhance it (such as normalizing the path so that '//' and '/'
are treated as equal for instance). But this paves the path for
further improvements of that sort. And in the meantime, if fixes
the problem we are facing.

For that, I added a new file, filename_cmp.c, updated the Makefile
to add this file to the list of required objects, and then changed
the FILENAME_CMP define to unconditionally call this new function.

libiberty/ change:
2007-03-28  Joel Brobecker  <brobecker@adacore.com>

        * filename_cmp.c: New file.
        * Makefile.in (CFILES): Add filename_cmp.c.
        (REQUIRED_OFILES): Add filename_cmp.o
        (filename_cmp.o): New rule.

include/ change:
2007-03-28  Joel Brobecker  <brobecker@adacore.com>

        * filenames.h (FILENAME_CMP): Adjust define to call filename_cmp
        regardless of the type of file system.

Tested on x86-linux and x86-windows, without any regression (using
the GDB testsuite).

OK to apply?

Thank you,
-- 
Joel

[-- Attachment #2: filename_cmp.c --]
[-- Type: text/plain, Size: 1959 bytes --]

/* File name comparison routine.
 *
   Copyright (C) 2007 Free Software Foundation, Inc.

   This file is part of the libiberty library, which is part of GCC.

   This file is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2 of the License, or
   (at your option) any later version.

   In addition to the permissions in the GNU General Public License, the
   Free Software Foundation gives you unlimited permission to link the
   compiled version of this file into combinations with other programs,
   and to distribute those combinations without any restriction coming
   from the use of this file.  (The General Public License restrictions
   do apply in other respects; for example, they cover modification of
   the file, and distribution when not linked into a combined
   executable.)

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. 
*/

#ifdef HAVE_STRING_H
#include <string.h>
#endif

#include <ctype.h>
#include "filenames.h"

int
filename_cmp (const char *s1, const char *s2)
{
#ifndef HAVE_DOS_BASED_FILE_SYSTEM
  return strcmp(s1, s2);
#else
  for (;;)
    {
      int c1 = tolower (*s1);
      int c2 = tolower (*s2);

      /* On DOS-based file systems, the '/' and the '\' are equivalent.  */
      if (c1 == '/')
        c1 = '\\';
      if (c2 == '/')
        c2 = '\\';

      if (c1 != c2)
        return (c1 - c2);

      if (c1 == '\0')
        return 0;

      s1++;
      s2++;
    }
#endif
}


[-- Attachment #3: filename_cmp.diff --]
[-- Type: text/plain, Size: 3385 bytes --]

Index: libiberty/Makefile.in
===================================================================
RCS file: /cvs/src/src/libiberty/Makefile.in,v
retrieving revision 1.85
diff -u -p -r1.85 Makefile.in
--- libiberty/Makefile.in	5 Mar 2007 22:15:38 -0000	1.85
+++ libiberty/Makefile.in	28 Mar 2007 19:16:41 -0000
@@ -1,6 +1,6 @@
 #
 # Makefile
-#   Copyright (C) 1990, 91-99, 2000, 2001, 2002, 2003, 2004, 2005, 2006
+#   Copyright (C) 1990, 91-99, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007
 #   Free Software Foundation
 #
 # This file is part of the libiberty library.
@@ -131,8 +131,8 @@ CFILES = alloca.c argv.c asprintf.c atex
 	calloc.c choose-temp.c clock.c concat.c cp-demangle.c		\
 	 cp-demint.c cplus-dem.c					\
 	dyn-string.c							\
-	fdmatch.c ffs.c fibheap.c floatformat.c fnmatch.c		\
-	 fopen_unlocked.c						\
+	fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c		\
+	fnmatch.c fopen_unlocked.c					\
 	getcwd.c getopt.c getopt1.c getpagesize.c getpwd.c getruntime.c	\
          gettimeofday.c                                                 \
 	hashtab.c hex.c							\
@@ -165,8 +165,8 @@ REQUIRED_OFILES = ./regex.o ./cplus-dem.
 	./alloca.o ./argv.o						\
 	./choose-temp.o ./concat.o ./cp-demint.o			\
 	./dyn-string.o							\
-	./fdmatch.o ./fibheap.o ./floatformat.o ./fnmatch.o		\
-	 ./fopen_unlocked.o						\
+	./fdmatch.o ./fibheap.o ./filename_cmp.o ./floatformat.o	\
+	./fnmatch.o ./fopen_unlocked.o					\
 	./getopt.o ./getopt1.o ./getpwd.o ./getruntime.o		\
 	./hashtab.o ./hex.o						\
 	./lbasename.o ./lrealpath.o					\
@@ -651,6 +651,12 @@ $(CONFIGURED_OFILES): stamp-picdir
 	else true; fi
 	$(COMPILE.c) $(srcdir)/fibheap.c $(OUTPUT_OPTION)
 
+./filename_cmp.o: $(srcdir)/filename_cmp.c $(INCDIR)/filenames.h
+	if [ x"$(PICFLAG)" != x ]; then \
+	  $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_cmp.c -o pic/$@; \
+	else true; fi
+	$(COMPILE.c) $(srcdir)/filename_cmp.c $(OUTPUT_OPTION)
+
 ./floatformat.o: $(srcdir)/floatformat.c config.h $(INCDIR)/ansidecl.h \
 	$(INCDIR)/floatformat.h $(INCDIR)/libiberty.h
 	if [ x"$(PICFLAG)" != x ]; then \
Index: include/filenames.h
===================================================================
RCS file: /cvs/src/src/include/filenames.h,v
retrieving revision 1.3
diff -u -p -r1.3 filenames.h
--- include/filenames.h	10 May 2005 10:21:08 -0000	1.3
+++ include/filenames.h	28 Mar 2007 19:16:41 -0000
@@ -5,7 +5,7 @@
    use forward- and back-slash in path names interchangeably, and
    some of them have case-insensitive file names.
 
-   Copyright 2000, 2001 Free Software Foundation, Inc.
+   Copyright 2000, 2001, 2007 Free Software Foundation, Inc.
 
 This file is part of BFD, the Binary File Descriptor library.
 
@@ -38,14 +38,15 @@ Foundation, Inc., 51 Franklin Street - F
    want to know whether to prepend the current working directory to
    a file name, which should not be done with a name like d:foo.  */
 #define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]) || (((f)[0]) && ((f)[1] == ':')))
-#define FILENAME_CMP(s1, s2)	strcasecmp(s1, s2)
 
 #else  /* not DOSish */
 
 #define IS_DIR_SEPARATOR(c)	((c) == '/')
 #define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]))
-#define FILENAME_CMP(s1, s2)	strcmp(s1, s2)
 
 #endif /* not DOSish */
 
+extern int filename_cmp (const char *s1, const char *s2);
+#define FILENAME_CMP(s1, s2)	filename_cmp(s1, s2)
+
 #endif /* FILENAMES_H */

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

[DJ Delorie]

Is this the right exception?  I think we want the gcc-only version,
like in vsprintf.c.  IIRC this one is for the files that are required
as part of the C++ ABI.

Unless this new file is used as part of the C++ runtime ABI?

>    In addition to the permissions in the GNU General Public License, the
>    Free Software Foundation gives you unlimited permission to link the
>    compiled version of this file into combinations with other programs,
>    and to distribute those combinations without any restriction coming
>    from the use of this file.  (The General Public License restrictions
>    do apply in other respects; for example, they cover modification of
>    the file, and distribution when not linked into a combined
>    executable.)

Also, please include inline documentation for functions.texi.

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

[Joel Brobecker]

> Is this the right exception?  I think we want the gcc-only version,
> like in vsprintf.c.  IIRC this one is for the files that are required
> as part of the C++ ABI.
> 
> Unless this new file is used as part of the C++ runtime ABI?

Hmmm, You are probably right. I copied this copyright blurb from
another file without paying enough attention. I think the physmem.c
copyright notice will be more appropriate, but I will double-check
where the macro is being used.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2, or (at your option)
   any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software Foundation,
   Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.  */

> Also, please include inline documentation for functions.texi.

Ah, yes, I forgot that part. Thanks for reminding me.

Thanks,
-- 
Joel

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

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

> Hmmm, You are probably right. I copied this copyright blurb from
> another file without paying enough attention. I think the physmem.c
> copyright notice will be more appropriate, but I will double-check
> where the macro is being used.

The macro is only used in java/jcf-path.c and protoize.c, so I don't
think it needs the exclusion.

Here is a revised version of the patch that should address both
comments (copyright notice, and lack of documentation):

libiberty/ change:
2007-03-28  Joel Brobecker  <brobecker@adacore.com>

        * filename_cmp.c: New file.
        * Makefile.in (CFILES): Add filename_cmp.c.
        (REQUIRED_OFILES): Add filename_cmp.o
        (filename_cmp.o): New rule.
        * functions.texi: Regenerate.

include/ change:
2007-03-28  Joel Brobecker  <brobecker@adacore.com>

        * filenames.h (FILENAME_CMP): Adjust define to call filename_cmp
        regardless of the type of file system.

The documentation change was tested by regenerating function.texi
then regenerating the .info documentation, and finally by browsing
the resulting document with the "info" program.

OK to apply?

Thank you,
-- 
Joel

[-- Attachment #2: filename_cmp.c --]
[-- Type: text/plain, Size: 2073 bytes --]

/* File name comparison routine.

   Copyright (C) 2007 Free Software Foundation, Inc.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2, or (at your option)
   any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software Foundation,
   Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.  */

#ifdef HAVE_STRING_H
#include <string.h>
#endif

#include <ctype.h>
#include "filenames.h"

/*

@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.
If not equivalent, the returned value is similar to what strcmp would
return.  In other words, it returns a negative value if @var{s1} is less
than @var{s2}, or a positive value if @var{s2} is greater than @var{s2}.

This function does not normalize path names. As a result, this function
will treat filenames that are spelled differently as different even in
the case when the two filenames point to the same underlying file.
However, it does handle the fact that on DOS-like file systems, forward
and backward slashes are equal.

@end deftypefn

*/

int
filename_cmp (const char *s1, const char *s2)
{
#ifndef HAVE_DOS_BASED_FILE_SYSTEM
  return strcmp(s1, s2);
#else
  for (;;)
    {
      int c1 = tolower (*s1);
      int c2 = tolower (*s2);

      /* On DOS-based file systems, the '/' and the '\' are equivalent.  */
      if (c1 == '/')
        c1 = '\\';
      if (c2 == '/')
        c2 = '\\';

      if (c1 != c2)
        return (c1 - c2);

      if (c1 == '\0')
        return 0;

      s1++;
      s2++;
    }
#endif
}


[-- Attachment #3: filename_cmp.diff --]
[-- Type: text/plain, Size: 4655 bytes --]

Index: libiberty/Makefile.in
===================================================================
RCS file: /cvs/src/src/libiberty/Makefile.in,v
retrieving revision 1.85
diff -u -p -r1.85 Makefile.in
--- libiberty/Makefile.in	5 Mar 2007 22:15:38 -0000	1.85
+++ libiberty/Makefile.in	28 Mar 2007 20:19:14 -0000
@@ -1,6 +1,6 @@
 #
 # Makefile
-#   Copyright (C) 1990, 91-99, 2000, 2001, 2002, 2003, 2004, 2005, 2006
+#   Copyright (C) 1990, 91-99, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007
 #   Free Software Foundation
 #
 # This file is part of the libiberty library.
@@ -131,8 +131,8 @@ CFILES = alloca.c argv.c asprintf.c atex
 	calloc.c choose-temp.c clock.c concat.c cp-demangle.c		\
 	 cp-demint.c cplus-dem.c					\
 	dyn-string.c							\
-	fdmatch.c ffs.c fibheap.c floatformat.c fnmatch.c		\
-	 fopen_unlocked.c						\
+	fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c		\
+	fnmatch.c fopen_unlocked.c					\
 	getcwd.c getopt.c getopt1.c getpagesize.c getpwd.c getruntime.c	\
          gettimeofday.c                                                 \
 	hashtab.c hex.c							\
@@ -165,8 +165,8 @@ REQUIRED_OFILES = ./regex.o ./cplus-dem.
 	./alloca.o ./argv.o						\
 	./choose-temp.o ./concat.o ./cp-demint.o			\
 	./dyn-string.o							\
-	./fdmatch.o ./fibheap.o ./floatformat.o ./fnmatch.o		\
-	 ./fopen_unlocked.o						\
+	./fdmatch.o ./fibheap.o ./filename_cmp.o ./floatformat.o	\
+	./fnmatch.o ./fopen_unlocked.o					\
 	./getopt.o ./getopt1.o ./getpwd.o ./getruntime.o		\
 	./hashtab.o ./hex.o						\
 	./lbasename.o ./lrealpath.o					\
@@ -651,6 +651,12 @@ $(CONFIGURED_OFILES): stamp-picdir
 	else true; fi
 	$(COMPILE.c) $(srcdir)/fibheap.c $(OUTPUT_OPTION)
 
+./filename_cmp.o: $(srcdir)/filename_cmp.c $(INCDIR)/filenames.h
+	if [ x"$(PICFLAG)" != x ]; then \
+	  $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_cmp.c -o pic/$@; \
+	else true; fi
+	$(COMPILE.c) $(srcdir)/filename_cmp.c $(OUTPUT_OPTION)
+
 ./floatformat.o: $(srcdir)/floatformat.c config.h $(INCDIR)/ansidecl.h \
 	$(INCDIR)/floatformat.h $(INCDIR)/libiberty.h
 	if [ x"$(PICFLAG)" != x ]; then \
Index: libiberty/functions.texi
===================================================================
RCS file: /cvs/src/src/libiberty/functions.texi,v
retrieving revision 1.30
diff -u -p -r1.30 functions.texi
--- libiberty/functions.texi	31 Jan 2007 20:25:23 -0000	1.30
+++ libiberty/functions.texi	28 Mar 2007 20:19:14 -0000
@@ -268,6 +268,22 @@ value 1).  If @var{valu} is zero, zero i
 
 @end deftypefn
 
+@c filename_cmp.c:28
+@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.
+If not equivalent, the returned value is similar to what strcmp would
+return.  In other words, it returns a negative value if @var{s1} is less
+than @var{s2}, or a positive value if @var{s2} is greater than @var{s2}.
+
+This function does not normalize path names. As a result, this function
+will treat filenames that are spelled differently as different even in
+the case when the two filenames point to the same underlying file.
+However, it does handle the fact that on DOS-like file systems, forward
+and backward slashes are equal.
+
+@end deftypefn
+
 @c fnmatch.txh:1
 @deftypefn Replacement int fnmatch (const char *@var{pattern}, const char *@var{string}, int @var{flags})
 
Index: include/filenames.h
===================================================================
RCS file: /cvs/src/src/include/filenames.h,v
retrieving revision 1.3
diff -u -p -r1.3 filenames.h
--- include/filenames.h	10 May 2005 10:21:08 -0000	1.3
+++ include/filenames.h	28 Mar 2007 20:19:14 -0000
@@ -5,7 +5,7 @@
    use forward- and back-slash in path names interchangeably, and
    some of them have case-insensitive file names.
 
-   Copyright 2000, 2001 Free Software Foundation, Inc.
+   Copyright 2000, 2001, 2007 Free Software Foundation, Inc.
 
 This file is part of BFD, the Binary File Descriptor library.
 
@@ -38,14 +38,15 @@ Foundation, Inc., 51 Franklin Street - F
    want to know whether to prepend the current working directory to
    a file name, which should not be done with a name like d:foo.  */
 #define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]) || (((f)[0]) && ((f)[1] == ':')))
-#define FILENAME_CMP(s1, s2)	strcasecmp(s1, s2)
 
 #else  /* not DOSish */
 
 #define IS_DIR_SEPARATOR(c)	((c) == '/')
 #define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]))
-#define FILENAME_CMP(s1, s2)	strcmp(s1, s2)
 
 #endif /* not DOSish */
 
+extern int filename_cmp (const char *s1, const char *s2);
+#define FILENAME_CMP(s1, s2)	filename_cmp(s1, s2)
+
 #endif /* FILENAMES_H */

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

[DJ Delorie]

Ok.

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

[Joel Brobecker]

> Ok.

Thank you. I just checked everything in.

-- 
Joel

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

[Joel Brobecker]

Re:

    libiberty/ change:
    2007-03-28  Joel Brobecker  <brobecker@adacore.com>
    
            * filename_cmp.c: New file.
            * Makefile.in (CFILES): Add filename_cmp.c.
            (REQUIRED_OFILES): Add filename_cmp.o
            (filename_cmp.o): New rule.
            * functions.texi: Regenerate.
    
    include/ change:
    2007-03-28  Joel Brobecker  <brobecker@adacore.com>
    
            * filenames.h (FILENAME_CMP): Adjust define to call filename_cmp
            regardless of the type of file system.

> > Ok.
> 
> Thank you. I just checked everything in.

I checked the change in gcc, but are these checkins propagated to GDB
as well? It doesn't seem like it.

How do we handle synchronization with libiberty changes?

-- 
Joel

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

[Daniel Jacobowitz]

On Thu, Mar 29, 2007 at 02:03:14PM -0700, Joel Brobecker wrote:
> > > Ok.
> > 
> > Thank you. I just checked everything in.
> 
> I checked the change in gcc, but are these checkins propagated to GDB
> as well? It doesn't seem like it.
> 
> How do we handle synchronization with libiberty changes?

Manually; please commit it to src too.

Thanks for fixing this, by the way.  We had a similar patch in our
queue of things to post...

-- 
Daniel Jacobowitz
CodeSourcery

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

[DJ Delorie]

> I checked the change in gcc, but are these checkins propagated to GDB
> as well? It doesn't seem like it.

Normally, my cron job does it within an hour, if I notice.  If you
commit to both, the cron job lets you ;-)

> How do we handle synchronization with libiberty changes?

gcc is the master.  Whenever gcc and src differ, my cron job tells me
and prepares a script that migrates the changes to src.  I just run
the script.

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

[Joel Brobecker]

> > I checked the change in gcc, but are these checkins propagated to GDB
> > as well? It doesn't seem like it.
> 
> Normally, my cron job does it within an hour, if I notice.  If you
> commit to both, the cron job lets you ;-)

Ah ha, I see now. Thanks! I will simply commit to gdb as well. 

-- 
Joel

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

[Eli Zaretskii]

> 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.

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

[Christopher Faylor]

>On Wed, 28 Mar 2007 12:18:45 -0700, Joel Brobaker wrote:
>>This is a modest improvement, since there are many other things we
>>can do to enhance it (such as normalizing the path so that '//' and '/'
>>are treated as equal for instance). But this paves the path for
>>further improvements of that sort.

...

On Sat, Mar 31, 2007 at 02:57:01PM +0300, Eli Zaretskii wrote:
>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?

Joel already acknowledge that this was a possible future improvement.

If this is done, please be sure to preserve two slashes at the beginning
of a filename since Windows uses those.

cgf

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

[Eli Zaretskii]

> Date: Sat, 31 Mar 2007 18:39:59 -0400
> From: Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>
> 
> On Sat, Mar 31, 2007 at 02:57:01PM +0300, Eli Zaretskii wrote:
> >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?
> 
> Joel already acknowledge that this was a possible future improvement.
> 
> If this is done, please be sure to preserve two slashes at the beginning
> of a filename since Windows uses those.

Right, sorry I forgot to mention that.

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

[Joel Brobecker]

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

Not a problem at all, Eli. I am not able to followup on them right
now, but I promise I will in a few days.

> >   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.

For this question, I'm not sure, actually.

-- 
Joel

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

[Andreas Schwab]

Joel Brobecker <brobecker@adacore.com> writes:

> /* File name comparison routine.
>
>    Copyright (C) 2007 Free Software Foundation, Inc.
>
>    This program is free software; you can redistribute it and/or modify
>    it under the terms of the GNU General Public License as published by
>    the Free Software Foundation; either version 2, or (at your option)
>    any later version.
>
>    This program is distributed in the hope that it will be useful,
>    but WITHOUT ANY WARRANTY; without even the implied warranty of
>    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>    GNU General Public License for more details.
>
>    You should have received a copy of the GNU General Public License
>    along with this program; if not, write to the Free Software Foundation,
>    Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.  */
>
> #ifdef HAVE_STRING_H
> #include <string.h>
> #endif

That won't work without #include "config.h".

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Joel Brobecker]

> > #ifdef HAVE_STRING_H
> > #include <string.h>
> > #endif
> 
> That won't work without #include "config.h".

Ah ha! Ben Elliston wrote me privately that he gets a warning
on x86-linux (Ubunty) that strcmp.h is not defined. That would
probably explain it. Thank you!

Unfortunately, I'm going to be unavailable for the next two of three
days. I'll try to fix it ASAP, but if someone could do this for me,
I would really appreciate it.

-- 
Joel

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

[Andreas Schwab]

Joel Brobecker <brobecker@adacore.com> writes:

>> > #ifdef HAVE_STRING_H
>> > #include <string.h>
>> > #endif
>> 
>> That won't work without #include "config.h".
>
> Ah ha! Ben Elliston wrote me privately that he gets a warning
> on x86-linux (Ubunty) that strcmp.h is not defined. That would
> probably explain it. Thank you!
>
> Unfortunately, I'm going to be unavailable for the next two of three
> days. I'll try to fix it ASAP, but if someone could do this for me,
> I would really appreciate it.

I've checked this in.

Andreas.

2007-04-02  Andreas Schwab  <schwab@suse.de>

	* filename_cmp.c: Include "config.h".

--- libiberty/filename_cmp.c.~1.1.~	2007-03-29 23:03:48.000000000 +0200
+++ libiberty/filename_cmp.c	2007-04-01 22:59:02.000000000 +0200
@@ -16,6 +16,10 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
 #ifdef HAVE_STRING_H
 #include <string.h>
 #endif

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."