[patch libiberty include]: Add additional helper functions for directory-separator searching

[patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

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

Hello,

This patch introduce directory-separator search routines to libiberty.
IMHO filename_cmp.c is suiteable for those functions, but if there are
objections about that I can move it into a separate source-file. It
helps to avoid a commonly used pattern about dir-separator searching
in code, which requires #if-conditions to check if DOS paths are used
and introduces additional internal variables.

the pattern

         const char *filename = strrchr (xloc.file, '/');
#ifdef HAVE_DOS_BASED_FILE_SYSTEM
         const char *filename2 = strrchr (xloc.file, '\\');

         if (!filename || (filename2 && filename2 > filename))
           filename = filename2;

can be written by this patch as
        const char *filename = filename_dirrchr (xloc.file);


ChangeLog include/

2011-03-08  Kai Tietz

	* filenames.h (filename_dirchr): New prototype.
	(filename_dirrchr): Likewise.

ChangeLog libiberty/

2011-03-08  Kai Tietz

	* filename_cmp.c (filename_dirchr): New function.
	(filename_dirrchr): Likewise.
	* functions.texi: Regenerated.


Tested for x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?

Regards,
Kai

[-- Attachment #2: libiberty_dirsep.txt --]
[-- Type: text/plain, Size: 3386 bytes --]

Index: gcc/include/filenames.h
===================================================================
--- gcc.orig/include/filenames.h	2011-02-28 19:16:35.000000000 +0100
+++ gcc/include/filenames.h	2011-03-08 11:11:10.909109700 +0100
@@ -75,6 +75,8 @@ extern int filename_cmp (const char *s1,
 
 extern int filename_ncmp (const char *s1, const char *s2,
 			  size_t n);
+extern char *filename_dirchr (const char *p);
+extern char *filename_dirrchr (const char *p);
 
 #ifdef __cplusplus
 }
Index: gcc/libiberty/filename_cmp.c
===================================================================
--- gcc.orig/libiberty/filename_cmp.c	2011-02-28 19:16:35.000000000 +0100
+++ gcc/libiberty/filename_cmp.c	2011-03-08 11:43:32.797198100 +0100
@@ -125,3 +125,70 @@ filename_ncmp (const char *s1, const cha
   return 0;
 #endif
 }
+
+/*
+
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirchr (const char *p)
+{
+  char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  char *r2;
+#endif
+  if (!p)
+    return NULL;
+  r = strchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  r2 = strchr (p, '\\');
+  if (!r || (r2 && r2 < r))
+    r = r2;
+#endif
+  return r;
+}
+
+/*
+
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirrchr (const char *p)
+{
+  char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  char *r2;
+#endif
+
+  if (!p)
+    return NULL;
+  r = strrchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  r2 = strrchr (p, '\\');
+  if (!r || (r2 && r2 > r))
+    r = r2;
+#endif
+  return r;
+}
Index: gcc/libiberty/functions.texi
===================================================================
--- gcc.orig/libiberty/functions.texi	2011-02-28 19:16:35.000000000 +0100
+++ gcc/libiberty/functions.texi	2011-03-08 11:43:42.314406700 +0100
@@ -296,6 +296,30 @@ and backward slashes are equal.
 
 @end deftypefn
 
+@c filename_cmp.c:131
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+@c filename_cmp.c:164
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
 @c filename_cmp.c:81
 @deftypefn Extension int filename_ncmp (const char *@var{s1}, const char *@var{s2}, size_t @var{n})
 

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> Date: Tue, 8 Mar 2011 11:56:45 +0100
> From: Kai Tietz <ktietz70@googlemail.com>
> 
> +@deftypefn Extension int filename_dirchr (const char *@var{p})
> +
> +The returned value is similar to what @code{strchr} would return for
> +searching for a directory separator.
> +
> +This function does not normalize file name.  However, it does handle
> +the fact that on DOS-like file systems, forward and backward slashes
> +are directory separators.

This is very mysterious.  The documentation should explain how this is
"handled", or else the user will have no choice but to look in the
sources.  And description "by similarity" doesn't help, because this
function is obviously different from strchr in _some_ ways, but you
don't say how.

While at that, explain the problem this solves, or else the raison
d'etre of this function will not be understood.  We do want this
function to be used instead of just strchr, don't we?  For it to be
used, its purpose and advantages should be well understood.

Btw, why do we need filename_dirchr?  The use case for
filename_dirrchr is clear, but in what situations will we need the
other one?

> +  if (!r || (r2 && r2 < r))

Why do you test for r2 being non-NULL?  You are not going to
dereference it in the next comparison, and NULL is comparable as any
other value.

> +@deftypefn Extension int filename_dirrchr (const char *@var{p})
> +
> +The returned value is similar to what @code{strrchr} would return for
> +searching for a directory separator.
> +
> +This function does not normalize file name.  However, it does handle
> +the fact that on DOS-like file systems, forward and backward slashes
> +are directory separators.

Same comments about this doc.

> +  if (!r || (r2 && r2 > r))

And same comment here about testing r2 for non-NULL value.

Please also wait for others to review, as I'm not authorized to
approve the changes.

Thanks for working on this.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

2011/3/8 Eli Zaretskii <eliz@gnu.org>:
>> Date: Tue, 8 Mar 2011 11:56:45 +0100
>> From: Kai Tietz <ktietz70@googlemail.com>
>>
>> +@deftypefn Extension int filename_dirchr (const char *@var{p})
>> +
>> +The returned value is similar to what @code{strchr} would return for
>> +searching for a directory separator.
>> +
>> +This function does not normalize file name.  However, it does handle
>> +the fact that on DOS-like file systems, forward and backward slashes
>> +are directory separators.
>
> This is very mysterious.  The documentation should explain how this is
> "handled", or else the user will have no choice but to look in the
> sources.  And description "by similarity" doesn't help, because this
> function is obviously different from strchr in _some_ ways, but you
> don't say how.
>
> While at that, explain the problem this solves, or else the raison
> d'etre of this function will not be understood.  We do want this
> function to be used instead of just strchr, don't we?  For it to be
> used, its purpose and advantages should be well understood.
>
> Btw, why do we need filename_dirchr?  The use case for
> filename_dirrchr is clear, but in what situations will we need the
> other one?

As the comment notes. strchr/strrchr searches for one character. This
is for unix-file-system normally slash. On DOS based file-systems
there are two characters representing a directory-separator. Slash and
Backslash. Therefore this routine takes care that both are handled
similiar to a single character searching.

>> +  if (!r || (r2 && r2 < r))
>
> Why do you test for r2 being non-NULL?  You are not going to
> dereference it in the next comparison, and NULL is comparable as any
> other value.

As if we found slash, we don't want to override function's result by
backslash not found. If the null-check wouldn't be present condition
would be always true for r2 == NULL as, NULL is always less then a
pointer. But r shall be modified only if r2 (backslash) was found
before r (slash).
(same logic but here from right to left for the strrchr-case)

Regards,
Kai

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> Date: Tue, 8 Mar 2011 12:25:37 +0100
> From: Kai Tietz <ktietz70@googlemail.com>
> Cc: binutils@sourceware.org, gdb-patches@sourceware.org, 
> 	gcc-patches@gcc.gnu.org
> 
> > Btw, why do we need filename_dirchr?  The use case for
> > filename_dirrchr is clear, but in what situations will we need the
> > other one?
> 
> As the comment notes. strchr/strrchr searches for one character. This
> is for unix-file-system normally slash. On DOS based file-systems
> there are two characters representing a directory-separator. Slash and
> Backslash. Therefore this routine takes care that both are handled
> similiar to a single character searching.

We are miscommunicating.  I was asking when would a program want to
find the _first_ directory separator character in a file name.
Searching for the last one is a frequent use case: when you want to
create a file in the same directory as another, or when you are
looking for a basename of a file.  But when do you need the first
slash?

> >> +  if (!r || (r2 && r2 < r))
> >
> > Why do you test for r2 being non-NULL?  You are not going to
> > dereference it in the next comparison, and NULL is comparable as any
> > other value.
> 
> As if we found slash, we don't want to override function's result by
> backslash not found. If the null-check wouldn't be present condition
> would be always true for r2 == NULL as, NULL is always less then a
> pointer. But r shall be modified only if r2 (backslash) was found
> before r (slash).
> (same logic but here from right to left for the strrchr-case)

But in strrchr-case, r2 cannot be greater than r1 if it is NULL,
right?

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

2011/3/8 Eli Zaretskii <eliz@gnu.org>:
>> Date: Tue, 8 Mar 2011 12:25:37 +0100
>> From: Kai Tietz <ktietz70@googlemail.com>
>> Cc: binutils@sourceware.org, gdb-patches@sourceware.org,
>>       gcc-patches@gcc.gnu.org
>>
>> > Btw, why do we need filename_dirchr?  The use case for
>> > filename_dirrchr is clear, but in what situations will we need the
>> > other one?
>>
>> As the comment notes. strchr/strrchr searches for one character. This
>> is for unix-file-system normally slash. On DOS based file-systems
>> there are two characters representing a directory-separator. Slash and
>> Backslash. Therefore this routine takes care that both are handled
>> similiar to a single character searching.
>
> We are miscommunicating.  I was asking when would a program want to
> find the _first_ directory separator character in a file name.
> Searching for the last one is a frequent use case: when you want to
> create a file in the same directory as another, or when you are
> looking for a basename of a file.  But when do you need the first
> slash?

See for example remote-fileio.c in remote_fileio_extract_ptr_w_len()
as an example. There is more then one use-case.

>> >> +  if (!r || (r2 && r2 < r))
>> >
>> > Why do you test for r2 being non-NULL?  You are not going to
>> > dereference it in the next comparison, and NULL is comparable as any
>> > other value.
>>
>> As if we found slash, we don't want to override function's result by
>> backslash not found. If the null-check wouldn't be present condition
>> would be always true for r2 == NULL as, NULL is always less then a
>> pointer. But r shall be modified only if r2 (backslash) was found
>> before r (slash).
>> (same logic but here from right to left for the strrchr-case)
>
> But in strrchr-case, r2 cannot be greater than r1 if it is NULL,
> right?
It can. It is a matter of signness of pointer comparision.

Regards,
Kai

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Pedro Alves]

On Tuesday 08 March 2011 12:01:24, Kai Tietz wrote:
> See for example remote-fileio.c in remote_fileio_extract_ptr_w_len()
> as an example. There is more then one use-case.

That '/' has nothing to do with path separators.  It's simply
a separator between a pointer and a length fields.  E.g.,

 @item Request:
 @samp{Fopen,@var{pathptr}/@var{len},@var{flags},@var{mode}}

@var{pathptr} is a pointer that points to the real path
in the inferior's memory, not a path string.

-- 
Pedro Alves

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

2011/3/8 Pedro Alves <pedro@codesourcery.com>:
> On Tuesday 08 March 2011 12:01:24, Kai Tietz wrote:
>> See for example remote-fileio.c in remote_fileio_extract_ptr_w_len()
>> as an example. There is more then one use-case.
>
> That '/' has nothing to do with path separators.  It's simply
> a separator between a pointer and a length fields.  E.g.,
>
>  @item Request:
>  @samp{Fopen,@var{pathptr}/@var{len},@var{flags},@var{mode}}
>
> @var{pathptr} is a pointer that points to the real path
> in the inferior's memory, not a path string.
>
> --
> Pedro Alves
>

Well, a better example is elfstab_offset_sections() in elfread.c.
Another is in find_file_and_directory() in dwarf2read.c file.

Regards,
Kai

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Pedro Alves]

On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote:

> Well, a better example is elfstab_offset_sections() in elfread.c.

  /* The ELF symbol info doesn't include path names, so strip the path
     (if any) from the psymtab filename.  */
  while (0 != (p = strchr (filename, '/')))
    filename = p + 1;

Looks like its looking for the last path separator, so
it might as well use filename_dirrchr instead.

> Another is in find_file_and_directory() in dwarf2read.c file.

Workaround for Irix.  Certainly that '/' should not depend
on the host gdb is running on.

-- 
Pedro Alves

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

2011/3/8 Pedro Alves <pedro@codesourcery.com>:
> On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote:
>
>> Well, a better example is elfstab_offset_sections() in elfread.c.
>
>  /* The ELF symbol info doesn't include path names, so strip the path
>     (if any) from the psymtab filename.  */
>  while (0 != (p = strchr (filename, '/')))
>    filename = p + 1;
>
> Looks like its looking for the last path separator, so
> it might as well use filename_dirrchr instead.

True, see patch I've posted about filename_cmp. I replaced it there by
a strrchr search.

>> Another is in find_file_and_directory() in dwarf2read.c file.
>
> Workaround for Irix.  Certainly that '/' should not depend
> on the host gdb is running on.

Right. But well, I was asked if strchr is used in combination with
paths. And so I've shown. If those uses could be rewritten is a
different story and might be true.

Kai

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 8 Mar 2011 13:33:02 +0000
> Cc: Kai Tietz <ktietz70@googlemail.com>,
>  gcc-patches@gcc.gnu.org,
>  Eli Zaretskii <eliz@gnu.org>,
>  binutils@sourceware.org
> 
> On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote:
> 
> > Well, a better example is elfstab_offset_sections() in elfread.c.
> 
>   /* The ELF symbol info doesn't include path names, so strip the path
>      (if any) from the psymtab filename.  */
>   while (0 != (p = strchr (filename, '/')))
>     filename = p + 1;
> 
> Looks like its looking for the last path separator, so
> it might as well use filename_dirrchr instead.

Exactly.

> > Another is in find_file_and_directory() in dwarf2read.c file.
> 
> Workaround for Irix.  Certainly that '/' should not depend
> on the host gdb is running on.

It actually should use IS_ABSOLUTE_FILE_NAME, if any portability
enhancement is needed here.

In my experience, the strchr analog is not needed, only the strrchr
one (which could be used quite a lot).  The few places that use strchr
now should actually be rewritten to search from the end, because
that's what they need.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Pedro Alves]

On Tuesday 08 March 2011 18:37:49, Eli Zaretskii wrote:
> > > Another is in find_file_and_directory() in dwarf2read.c file.
> > 
> > Workaround for Irix.  Certainly that '/' should not depend
> > on the host gdb is running on.
> 
> It actually should use IS_ABSOLUTE_FILE_NAME, if any portability
> enhancement is needed here.

The point of the code, according to its comment,
is to workaround an issue with the debug info output by the
native Irix compiler.  You wouldn't want a cross-Irix,
Windows-hosted gdb looking for '\' or a drive prefix in order
to decide whether to apply the workaround.  In other words,
we _always_ want to check for literal '/' here:

  if (*comp_dir != NULL)
    {
      /* Irix 6.2 native cc prepends <machine>.: to the compilation
	 directory, get rid of it.  */
      char *cp = strchr (*comp_dir, ':');

      if (cp && cp != *comp_dir && cp[-1] == '.' && cp[1] == '/')
	*comp_dir = cp + 1;
    }

-- 
Pedro Alves

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

2011/3/8 Eli Zaretskii <eliz@gnu.org>:
>> From: Pedro Alves <pedro@codesourcery.com>
>> Date: Tue, 8 Mar 2011 13:33:02 +0000
>> Cc: Kai Tietz <ktietz70@googlemail.com>,
>>  gcc-patches@gcc.gnu.org,
>>  Eli Zaretskii <eliz@gnu.org>,
>>  binutils@sourceware.org
>>
>> On Tuesday 08 March 2011 12:48:11, Kai Tietz wrote:
>>
>> > Well, a better example is elfstab_offset_sections() in elfread.c.
>>
>>   /* The ELF symbol info doesn't include path names, so strip the path
>>      (if any) from the psymtab filename.  */
>>   while (0 != (p = strchr (filename, '/')))
>>     filename = p + 1;
>>
>> Looks like its looking for the last path separator, so
>> it might as well use filename_dirrchr instead.
>
> Exactly.
>
>> > Another is in find_file_and_directory() in dwarf2read.c file.
>>
>> Workaround for Irix.  Certainly that '/' should not depend
>> on the host gdb is running on.
>
> It actually should use IS_ABSOLUTE_FILE_NAME, if any portability
> enhancement is needed here.
>
> In my experience, the strchr analog is not needed, only the strrchr
> one (which could be used quite a lot).  The few places that use strchr
> now should actually be rewritten to search from the end, because
> that's what they need.
>

Here I am not that sure. For example in gcc's gengtype.c
(read_input_list) is a use-case for strchr on filenames, which can't
be expressed by strrchr.
Please be aware that libiberty is shared between different ventures.

I admit that filenames/paths are searched normal from right to left.
But there might be cases a left to right search is suitable.

Regards,
Kai

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> Date: Tue, 8 Mar 2011 19:51:14 +0100
> From: Kai Tietz <ktietz70@googlemail.com>
> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org, 
> 	gcc-patches@gcc.gnu.org, binutils@sourceware.org
> 
> > In my experience, the strchr analog is not needed, only the strrchr
> > one (which could be used quite a lot).  The few places that use strchr
> > now should actually be rewritten to search from the end, because
> > that's what they need.
> >
> 
> Here I am not that sure. For example in gcc's gengtype.c
> (read_input_list) is a use-case for strchr on filenames, which can't
> be expressed by strrchr.

I don't see any reason to have in libiberty a function that has a
single use.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> NULL is comparable as any other value.

Only for equality.  For relational operators the operands must point to
the same object.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

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

Hi,

I update the patch and put those new function into a separate file.

ChangeLog include/

2011-03-08  Kai Tietz

	* filenames.h (filename_dirchr): New prototype.
	(filename_dirrchr): Likewise.

ChangeLog libiberty/

2011-03-08  Kai Tietz

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

Tested for x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?

Regards,
Kai

[-- Attachment #2: libiberty_dirsep.txt --]
[-- Type: text/plain, Size: 6163 bytes --]

Index: gcc/include/filenames.h
===================================================================
--- gcc.orig/include/filenames.h	2011-02-28 19:16:35.000000000 +0100
+++ gcc/include/filenames.h	2011-03-08 11:11:10.909109700 +0100
@@ -75,6 +75,8 @@ extern int filename_cmp (const char *s1,
 
 extern int filename_ncmp (const char *s1, const char *s2,
 			  size_t n);
+extern char *filename_dirchr (const char *p);
+extern char *filename_dirrchr (const char *p);
 
 #ifdef __cplusplus
 }
Index: gcc/libiberty/functions.texi
===================================================================
--- gcc.orig/libiberty/functions.texi	2011-02-28 19:16:35.000000000 +0100
+++ gcc/libiberty/functions.texi	2011-03-08 16:02:12.147905400 +0100
@@ -296,6 +296,30 @@ and backward slashes are equal.
 
 @end deftypefn
 
+@c filename_chr.c:32
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+@c filename_chr.c:65
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
 @c filename_cmp.c:81
 @deftypefn Extension int filename_ncmp (const char *@var{s1}, const char *@var{s2}, size_t @var{n})
 
Index: gcc/libiberty/Makefile.in
===================================================================
--- gcc.orig/libiberty/Makefile.in	2010-11-21 14:28:05.000000000 +0100
+++ gcc/libiberty/Makefile.in	2011-03-08 16:01:09.254418900 +0100
@@ -127,8 +127,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 crc32.c				\
 	dyn-string.c							\
-	fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c		\
-	fnmatch.c fopen_unlocked.c					\
+	fdmatch.c ffs.c fibheap.c filename_cmp.c filename_chr.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							\
@@ -168,7 +168,8 @@ REQUIRED_OFILES =							\
 	./choose-temp.$(objext) ./concat.$(objext)			\
 	./cp-demint.$(objext) ./crc32.$(objext) ./dyn-string.$(objext)	\
 	./fdmatch.$(objext) ./fibheap.$(objext)				\
-	./filename_cmp.$(objext) ./floatformat.$(objext)		\
+	./filename_cmp.$(objext) ./filename_chr.$(objext)		\
+	./floatformat.$(objext)						\
 	./fnmatch.$(objext) ./fopen_unlocked.$(objext)			\
 	./getopt.$(objext) ./getopt1.$(objext) ./getpwd.$(objext)	\
 	./getruntime.$(objext) ./hashtab.$(objext) ./hex.$(objext)	\
@@ -653,6 +654,13 @@ $(CONFIGURED_OFILES): stamp-picdir
 	else true; fi
 	$(COMPILE.c) $(srcdir)/filename_cmp.c $(OUTPUT_OPTION)
 
+./filename_chr.$(objext): $(srcdir)/filename_chr.c config.h $(INCDIR)/filenames.h \
+	$(INCDIR)/safe-ctype.h
+	if [ x"$(PICFLAG)" != x ]; then \
+	  $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_chr.c -o pic/$@; \
+	else true; fi
+	$(COMPILE.c) $(srcdir)/filename_chr.c $(OUTPUT_OPTION)
+
 ./floatformat.$(objext): $(srcdir)/floatformat.c config.h $(INCDIR)/ansidecl.h \
 	$(INCDIR)/floatformat.h $(INCDIR)/libiberty.h
 	if [ x"$(PICFLAG)" != x ]; then \
Index: gcc/libiberty/filename_chr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/libiberty/filename_chr.c	2011-03-08 15:55:30.844446300 +0100
@@ -0,0 +1,95 @@
+/* File name comparison routine.
+
+   Copyright (C) 2011 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_CONFIG_H
+#include "config.h"
+#endif
+
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
+
+#include "filenames.h"
+#include "safe-ctype.h"
+
+/*
+
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirchr (const char *p)
+{
+  char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  char *r2;
+#endif
+  if (!p)
+    return NULL;
+  r = strchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  r2 = strchr (p, '\\');
+  if (!r || (r2 && r2 < r))
+    r = r2;
+#endif
+  return r;
+}
+
+/*
+
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirrchr (const char *p)
+{
+  char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  char *r2;
+#endif
+
+  if (!p)
+    return NULL;
+  r = strrchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  r2 = strrchr (p, '\\');
+  if (!r || (r2 && r2 > r))
+    r = r2;
+#endif
+  return r;
+}

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

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

Umm, sorry. I found a wrong copy & paste. So I re-sent the corrected
patch. Additionally I adjuste the changes in Makefile.in so, that
alphabetic order remains.

Kai

[-- Attachment #2: libiberty_dirsep.txt --]
[-- Type: text/plain, Size: 6148 bytes --]

Index: gcc/include/filenames.h
===================================================================
--- gcc.orig/include/filenames.h	2011-02-28 19:16:35.000000000 +0100
+++ gcc/include/filenames.h	2011-03-08 11:11:10.909109700 +0100
@@ -75,6 +75,8 @@ extern int filename_cmp (const char *s1,
 
 extern int filename_ncmp (const char *s1, const char *s2,
 			  size_t n);
+extern char *filename_dirchr (const char *p);
+extern char *filename_dirrchr (const char *p);
 
 #ifdef __cplusplus
 }
Index: gcc/libiberty/functions.texi
===================================================================
--- gcc.orig/libiberty/functions.texi	2011-02-28 19:16:35.000000000 +0100
+++ gcc/libiberty/functions.texi	2011-03-08 16:26:29.547971700 +0100
@@ -296,6 +296,30 @@ and backward slashes are equal.
 
 @end deftypefn
 
+@c filename_chr.c:32
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+@c filename_chr.c:65
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
 @c filename_cmp.c:81
 @deftypefn Extension int filename_ncmp (const char *@var{s1}, const char *@var{s2}, size_t @var{n})
 
Index: gcc/libiberty/Makefile.in
===================================================================
--- gcc.orig/libiberty/Makefile.in	2010-11-21 14:28:05.000000000 +0100
+++ gcc/libiberty/Makefile.in	2011-03-08 16:24:17.703229600 +0100
@@ -127,8 +127,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 crc32.c				\
 	dyn-string.c							\
-	fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c		\
-	fnmatch.c fopen_unlocked.c					\
+	fdmatch.c ffs.c fibheap.c filename_chr.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							\
@@ -168,7 +168,8 @@ REQUIRED_OFILES =							\
 	./choose-temp.$(objext) ./concat.$(objext)			\
 	./cp-demint.$(objext) ./crc32.$(objext) ./dyn-string.$(objext)	\
 	./fdmatch.$(objext) ./fibheap.$(objext)				\
-	./filename_cmp.$(objext) ./floatformat.$(objext)		\
+	./filename_chr.$(objext) ./filename_cmp.$(objext)		\
+	./floatformat.$(objext)						\
 	./fnmatch.$(objext) ./fopen_unlocked.$(objext)			\
 	./getopt.$(objext) ./getopt1.$(objext) ./getpwd.$(objext)	\
 	./getruntime.$(objext) ./hashtab.$(objext) ./hex.$(objext)	\
@@ -646,6 +647,13 @@ $(CONFIGURED_OFILES): stamp-picdir
 	else true; fi
 	$(COMPILE.c) $(srcdir)/fibheap.c $(OUTPUT_OPTION)
 
+./filename_chr.$(objext): $(srcdir)/filename_chr.c config.h $(INCDIR)/filenames.h \
+	$(INCDIR)/safe-ctype.h
+	if [ x"$(PICFLAG)" != x ]; then \
+	  $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_chr.c -o pic/$@; \
+	else true; fi
+	$(COMPILE.c) $(srcdir)/filename_chr.c $(OUTPUT_OPTION)
+
 ./filename_cmp.$(objext): $(srcdir)/filename_cmp.c config.h $(INCDIR)/filenames.h \
 	$(INCDIR)/safe-ctype.h
 	if [ x"$(PICFLAG)" != x ]; then \
Index: gcc/libiberty/filename_chr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/libiberty/filename_chr.c	2011-03-08 16:22:51.303258200 +0100
@@ -0,0 +1,95 @@
+/* File name character searching routines.
+
+   Copyright (C) 2011 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_CONFIG_H
+#include "config.h"
+#endif
+
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
+
+#include "filenames.h"
+#include "safe-ctype.h"
+
+/*
+
+@deftypefn Extension int filename_dirchr (const char *@var{p})
+
+The returned value is similar to what @code{strchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirchr (const char *p)
+{
+  char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  char *r2;
+#endif
+  if (!p)
+    return NULL;
+  r = strchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  r2 = strchr (p, '\\');
+  if (!r || (r2 && r2 < r))
+    r = r2;
+#endif
+  return r;
+}
+
+/*
+
+@deftypefn Extension int filename_dirrchr (const char *@var{p})
+
+The returned value is similar to what @code{strrchr} would return for
+searching for a directory separator.
+
+This function does not normalize file name.  However, it does handle
+the fact that on DOS-like file systems, forward and backward slashes
+are directory separators.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirrchr (const char *p)
+{
+  char *r;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  char *r2;
+#endif
+
+  if (!p)
+    return NULL;
+  r = strrchr (p, '/');
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+  r2 = strrchr (p, '\\');
+  if (!r || (r2 && r2 > r))
+    r = r2;
+#endif
+  return r;
+}

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> Date: Tue, 8 Mar 2011 16:29:51 +0100
> From: Kai Tietz <ktietz70@googlemail.com>
> 
> Umm, sorry. I found a wrong copy & paste. So I re-sent the corrected
> patch. Additionally I adjuste the changes in Makefile.in so, that
> alphabetic order remains.

I think we don't need filename_dirchr, only filename_dirrchr.

And you didn't change anything in the documentation to address my
comments earlier today.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[DJ Delorie]

> I think we don't need filename_dirchr, only filename_dirrchr.

I see no harm in having both, for completeness, though.  One could
argue they should be in separate files, but they're trivial functions
on non-dos-fs systems.

What bothers me about this patch is that we're adding yet another set
of functions that don't discriminate between the host filesystem, the
target filesystem, and the build filesystem.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> Date: Tue, 8 Mar 2011 14:41:00 -0500
> From: DJ Delorie <dj@redhat.com>
> CC: ktietz70@googlemail.com, binutils@sourceware.org,        gdb-patches@sourceware.org, gcc-patches@gcc.gnu.org
> 
> 
> I see no harm in having both, for completeness, though.

I don't see any need for completeness in this case.  But it's your
call.  I still think that the documentation should be fixed, though.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Pedro Alves]

Actually, is there any case where lbasename wouldn't
work instead of filename_dirrchr?

(gdb is already making use of 
unix_lbasename / dos_lbasename at places where it
needs to handle host vs target paths).

-- 
Pedro Alves

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 8 Mar 2011 22:37:59 +0000
> Cc: DJ Delorie <dj@redhat.com>,
>  Eli Zaretskii <eliz@gnu.org>,
>  ktietz70@googlemail.com,
>  binutils@sourceware.org,
>  gcc-patches@gcc.gnu.org
> 
> Actually, is there any case where lbasename wouldn't
> work instead of filename_dirrchr?

Almost: lbasename returns a pointer one character _after_ the last
slash.  It also skips the drive letter on DOS/Windows (which might be
TRT, actually).

It would be reasonable to rewrite filename_dirrchr in terms of
lbasename, though, by backing up the pointer returned by lbasename if
it points to a slash, and otherwise returning NULL.  The case of
"d:foo" should also be resolved (probably, return a pointer to the
colon).

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Pedro Alves]

On Wednesday 09 March 2011 05:29:09, Eli Zaretskii wrote:
> > Actually, is there any case where lbasename wouldn't
> > work instead of filename_dirrchr?
> 
> Almost: lbasename returns a pointer one character after the last
> slash.  It also skips the drive letter on DOS/Windows (which might be
> TRT, actually).

I meant a valid use case in the code bases.  Might as
well cook up a (gdb) patch.  Find it pasted below.  Does it look good to you?

The one's left are: 1 in a linux-native only file (never cares
for other filesystem semantics), and a couple in the coff and
mdebug readers.  The latter could be rewritten in terms of
lbasename, but I'm not sure whether gcc outputs a literal '/' in
that case even when building on mingw.  If so, and we changed them,
we'd be breaking reading these files on Windows, so I'd rather
change them only if proven necessary.  (remember the source vs host
path distinction we're missing)

> It would be reasonable to rewrite filename_dirrchr in terms of
> lbasename, though, by backing up the pointer returned by lbasename if
> it points to a slash, and otherwise returning NULL.  The case of
> "d:foo" should also be resolved (probably, return a pointer to the
> colon).

The means it's just an adapter, and callers could be rewritten
a little to think in terms of lbasename.  The only case where
I imagine filename_dirrchr would be a little bit more efficient,
is when breaking up path directories components from right
to left, for step-wise comparison with another path, say.
IMO, the less code to maintain and the fewer the APIs
readers have to understand, the better.

I did a:

 grep strrchr * | grep "'/'"

under gcc and all the cases I saw could/should
be using lbasename.  I suggest gcc folks fix that first
before anything else.

-- 
Pedro Alves

2011-03-09  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* cli/cli-cmds.c (shell_escape): Use lbasename.
	* coffread.c (coff_start_symtab): Constify parameter.
	(complete_symtab): Constify `name' parameter.
	(coff_symtab_read): Constify `filestring' local.
	(coff_getfilename): Constify return and `result' local.
	Use lbasename.
	* fbsd-nat.c (fbsd_make_corefile_notes): Use lbasename.
	* linux-fork.c (info_checkpoints_command): Use lbasename.
	* linux-nat.c (linux_nat_make_corefile_notes): Use lbasename.
	* minsyms.c (lookup_minimal_symbol): Use lbasename.
	* nto-tdep.c (nto_find_and_open_solib): Use lbasename.
	* procfs.c (procfs_make_note_section): Use lbasename.
	* tui/tui-io.c (printable_part): Constity return and parameter.
	Use lbasename.
	(print_filename): Constify parameters, and local `s'.
	(tui_rl_display_match_list): Constify local `temp'.

---
 gdb/cli/cli-cmds.c |    7 ++-----
 gdb/coffread.c     |   15 +++++++--------
 gdb/fbsd-nat.c     |    2 +-
 gdb/linux-fork.c   |    9 +--------
 gdb/linux-nat.c    |    2 +-
 gdb/minsyms.c      |    7 +------
 gdb/nto-tdep.c     |    8 +-------
 gdb/procfs.c       |    2 +-
 gdb/tui/tui-io.c   |   22 ++++++----------------
 9 files changed, 21 insertions(+), 53 deletions(-)

Index: src/gdb/cli/cli-cmds.c
===================================================================
--- src.orig/gdb/cli/cli-cmds.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/cli/cli-cmds.c	2011-03-09 11:14:37.672800007 +0000
@@ -730,16 +730,13 @@ shell_escape (char *arg, int from_tty)
 
   if ((pid = vfork ()) == 0)
     {
-      char *p, *user_shell;
+      const char *p, *user_shell;
 
       if ((user_shell = (char *) getenv ("SHELL")) == NULL)
 	user_shell = "/bin/sh";
 
       /* Get the name of the shell for arg0.  */
-      if ((p = strrchr (user_shell, '/')) == NULL)
-	p = user_shell;
-      else
-	p++;			/* Get past '/' */
+      p = lbasename (user_shell);
 
       if (!arg)
 	execl (user_shell, p, (char *) 0);
Index: src/gdb/coffread.c
===================================================================
--- src.orig/gdb/coffread.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/coffread.c	2011-03-09 11:14:37.672800007 +0000
@@ -178,7 +178,7 @@ static int init_lineno (bfd *, long, int
 
 static char *getsymname (struct internal_syment *);
 
-static char *coff_getfilename (union internal_auxent *);
+static const char *coff_getfilename (union internal_auxent *);
 
 static void free_stringtab (void);
 
@@ -366,7 +366,7 @@ coff_alloc_type (int index)
    it indicates the start of data for one original source file.  */
 
 static void
-coff_start_symtab (char *name)
+coff_start_symtab (const char *name)
 {
   start_symtab (
   /* We fill in the filename later.  start_symtab puts this pointer
@@ -388,7 +388,7 @@ coff_start_symtab (char *name)
    text.  */
 
 static void
-complete_symtab (char *name, CORE_ADDR start_addr, unsigned int size)
+complete_symtab (const char *name, CORE_ADDR start_addr, unsigned int size)
 {
   if (last_source_file != NULL)
     xfree (last_source_file);
@@ -713,7 +713,7 @@ coff_symtab_read (long symtab_offset, un
   int in_source_file = 0;
   int next_file_symnum = -1;
   /* Name of the current file.  */
-  char *filestring = "";
+  const char *filestring = "";
   int depth = 0;
   int fcn_first_line = 0;
   CORE_ADDR fcn_first_line_addr = 0;
@@ -1308,12 +1308,12 @@ getsymname (struct internal_syment *symb
    Return only the last component of the name.  Result is in static
    storage and is only good for temporary use.  */
 
-static char *
+static const char *
 coff_getfilename (union internal_auxent *aux_entry)
 {
   static char buffer[BUFSIZ];
   char *temp;
-  char *result;
+  const char *result;
 
   if (aux_entry->x_file.x_n.x_zeroes == 0)
     {
@@ -1331,8 +1331,7 @@ coff_getfilename (union internal_auxent
   /* FIXME: We should not be throwing away the information about what
      directory.  It should go into dirname of the symtab, or some such
      place.  */
-  if ((temp = strrchr (result, '/')) != NULL)
-    result = temp + 1;
+  result = lbasename (result);
   return (result);
 }
 \f
Index: src/gdb/fbsd-nat.c
===================================================================
--- src.orig/gdb/fbsd-nat.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/fbsd-nat.c	2011-03-09 11:14:37.682800003 +0000
@@ -202,7 +202,7 @@ fbsd_make_corefile_notes (bfd *obfd, int
 
   if (get_exec_file (0))
     {
-      char *fname = strrchr (get_exec_file (0), '/') + 1;
+      char *fname = lbasename (get_exec_file (0));
       char *psargs = xstrdup (fname);
 
       if (get_inferior_args ())
Index: src/gdb/linux-fork.c
===================================================================
--- src.orig/gdb/linux-fork.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/linux-fork.c	2011-03-09 11:14:37.682800003 +0000
@@ -584,14 +584,7 @@ info_checkpoints_command (char *arg, int
 
       sal = find_pc_line (pc, 0);
       if (sal.symtab)
-	{
-	  char *tmp = strrchr (sal.symtab->filename, '/');
-
-	  if (tmp)
-	    printf_filtered (_(", file %s"), tmp + 1);
-	  else
-	    printf_filtered (_(", file %s"), sal.symtab->filename);
-	}
+	printf_filtered (_(", file %s"), lbasename (sal.symtab->filename));
       if (sal.line)
 	printf_filtered (_(", line %d"), sal.line);
       if (!sal.symtab && !sal.line)
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/linux-nat.c	2011-03-09 11:14:37.682800003 +0000
@@ -4491,7 +4491,7 @@ linux_nat_make_corefile_notes (bfd *obfd
 
   if (get_exec_file (0))
     {
-      strncpy (fname, strrchr (get_exec_file (0), '/') + 1, sizeof (fname));
+      strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname));
       strncpy (psargs, get_exec_file (0), sizeof (psargs));
       if (get_inferior_args ())
 	{
Index: src/gdb/minsyms.c
===================================================================
--- src.orig/gdb/minsyms.c	2011-03-09 10:53:22.072800005 +0000
+++ src/gdb/minsyms.c	2011-03-09 11:14:37.682800003 +0000
@@ -198,12 +198,7 @@ lookup_minimal_symbol (const char *name,
   const char *modified_name;
 
   if (sfile != NULL)
-    {
-      char *p = strrchr (sfile, '/');
-
-      if (p != NULL)
-	sfile = p + 1;
-    }
+    sfile = lbasename (sfile);
 
   /* For C++, canonicalize the input name.  */
   modified_name = name;
Index: src/gdb/nto-tdep.c
===================================================================
--- src.orig/gdb/nto-tdep.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/nto-tdep.c	2011-03-09 11:14:37.682800003 +0000
@@ -127,13 +127,7 @@ nto_find_and_open_solib (char *solib, un
   sprintf (buf, PATH_FMT, arch_path, arch_path, arch_path, arch_path,
 	   arch_path);
 
-  /* Don't assume basename() isn't destructive.  */
-  base = strrchr (solib, '/');
-  if (!base)
-    base = solib;
-  else
-    base++;			/* Skip over '/'.  */
-
+  base = lbasename (solib);
   ret = openp (buf, 1, base, o_flags, temp_pathname);
   if (ret < 0 && base != solib)
     {
Index: src/gdb/procfs.c
===================================================================
--- src.orig/gdb/procfs.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/procfs.c	2011-03-09 11:14:37.682800003 +0000
@@ -5734,7 +5734,7 @@ procfs_make_note_section (bfd *obfd, int
 
   if (get_exec_file (0))
     {
-      strncpy (fname, strrchr (get_exec_file (0), '/') + 1, sizeof (fname));
+      strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname));
       strncpy (psargs, get_exec_file (0),
 	       sizeof (psargs));
 
Index: src/gdb/tui/tui-io.c
===================================================================
--- src.orig/gdb/tui/tui-io.c	2011-03-09 10:53:22.062800003 +0000
+++ src/gdb/tui/tui-io.c	2011-03-09 11:14:37.682800003 +0000
@@ -324,20 +324,10 @@ tui_readline_output (int error, gdb_clie
    final slash.  Otherwise, we return what we were passed.
 
    Comes from readline/complete.c.  */
-static char *
-printable_part (char *pathname)
+static const char *
+printable_part (const char *pathname)
 {
-  char *temp;
-
-  temp = rl_filename_completion_desired
-    ? strrchr (pathname, '/') : (char *)NULL;
-#if defined (__MSDOS__)
-  if (rl_filename_completion_desired 
-      && temp == 0 && isalpha (pathname[0]) 
-      && pathname[1] == ':')
-    temp = pathname + 1;
-#endif
-  return (temp ? ++temp : pathname);
+  return rl_filename_completion_desired ? lbasename (pathname) : pathname;
 }
 
 /* Output TO_PRINT to rl_outstream.  If VISIBLE_STATS is defined and
@@ -366,10 +356,10 @@ printable_part (char *pathname)
     } while (0)
 
 static int
-print_filename (char *to_print, char *full_pathname)
+print_filename (const char *to_print, const char *full_pathname)
 {
   int printed_len = 0;
-  char *s;
+  const char *s;
 
   for (s = to_print; *s; s++)
     {
@@ -416,7 +406,7 @@ tui_rl_display_match_list (char **matche
   
   int count, limit, printed_len;
   int i, j, k, l;
-  char *temp;
+  const char *temp;
 
   /* Screen dimension correspond to the TUI command window.  */
   int screenwidth = TUI_CMD_WIN->generic.width;

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 9 Mar 2011 11:46:36 +0000
> Cc: gdb-patches@sourceware.org,
>  dj@redhat.com,
>  ktietz70@googlemail.com,
>  binutils@sourceware.org,
>  gcc-patches@gcc.gnu.org
> 
> On Wednesday 09 March 2011 05:29:09, Eli Zaretskii wrote:
> > > Actually, is there any case where lbasename wouldn't
> > > work instead of filename_dirrchr?
> > 
> > Almost: lbasename returns a pointer one character after the last
> > slash.  It also skips the drive letter on DOS/Windows (which might be
> > TRT, actually).
> 
> I meant a valid use case in the code bases.

Sorry for my misunderstanding.

> Might as well cook up a (gdb) patch.  Find it pasted below.  Does it
> look good to you?

Yes, looks fine.  Thanks.

> The one's left are: 1 in a linux-native only file (never cares
> for other filesystem semantics), and a couple in the coff and
> mdebug readers.  The latter could be rewritten in terms of
> lbasename, but I'm not sure whether gcc outputs a literal '/' in
> that case even when building on mingw.  If so, and we changed them,
> we'd be breaking reading these files on Windows

Sorry, I don't understand how would that break on Windows.  Could you
elaborate?  And what "couple of coff and mdebug readers" did you have
in mind?

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Pedro Alves]

On Wednesday 09 March 2011 12:35:00, Eli Zaretskii wrote:
> > I meant a valid use case in the code bases.
> 
> Sorry for my misunderstanding.

NP.

> 
> > Might as well cook up a (gdb) patch.  Find it pasted below.  Does it
> > look good to you?
> 
> Yes, looks fine.  Thanks.

Thanks.  I've applied it.

> > The one's left are: 1 in a linux-native only file (never cares
> > for other filesystem semantics), and a couple in the coff and
> > mdebug readers.  The latter could be rewritten in terms of
> > lbasename, but I'm not sure whether gcc outputs a literal '/' in
> > that case even when building on mingw.  If so, and we changed them,
> > we'd be breaking reading these files on Windows
> 
> Sorry, I don't understand how would that break on Windows.  Could you
> elaborate?  And what "couple of coff and mdebug readers" did you have
> in mind?

Sorry, in the hurry, I had a (another) brain cramp.  Wouldn't break.
Still it'd be useless to change this _if_ gcc hardcodes '/'.  Dunno
whether it does.

>find . -name "*.c" | xargs grep strrchr | grep "'/'"
./linux-thread-db.c:    cp = strrchr (path, '/');
./mdebugread.c:               p = strrchr (namestring, '/');
./dbxread.c:        p = strrchr (namestring, '/');

Both look like this:

	    /* Some compilers (including gcc) emit a pair of initial N_SOs.
	       The first one is a directory name; the second the file name.
	       If pst exists, is empty, and has a filename ending in '/',
	       we assume the previous N_SO was a directory name.  */

	    p = strrchr (namestring, '/');
	    if (p && *(p + 1) == '\000')
	      {
		/* Save the directory name SOs locally, then save it into
		   the psymtab when it's created below.  */
	        dirname_nso = namestring;
	        continue;		
	      }

-- 
Pedro Alves

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 9 Mar 2011 12:58:38 +0000
> Cc: gdb-patches@sourceware.org,
>  dj@redhat.com,
>  ktietz70@googlemail.com,
>  binutils@sourceware.org,
>  gcc-patches@gcc.gnu.org
> 
> > > The one's left are: 1 in a linux-native only file (never cares
> > > for other filesystem semantics), and a couple in the coff and
> > > mdebug readers.  The latter could be rewritten in terms of
> > > lbasename, but I'm not sure whether gcc outputs a literal '/' in
> > > that case even when building on mingw.  If so, and we changed them,
> > > we'd be breaking reading these files on Windows
> > 
> > Sorry, I don't understand how would that break on Windows.  Could you
> > elaborate?  And what "couple of coff and mdebug readers" did you have
> > in mind?
> 
> Sorry, in the hurry, I had a (another) brain cramp.  Wouldn't break.
> Still it'd be useless to change this _if_ gcc hardcodes '/'.  Dunno
> whether it does.

At least on MinGW, GCC simply uses whatever was passed on the command
line.  I tested that by compiling the same source file, passing it to
GCC with different flavors of slashes, including mixed ones.  Then in
GDB I typed "info sources" and saw the source file with exactly the
same flavor of slashes as what I typed on the GCC command line.

Funnily enough, when the file name given to GCC includes at least one
backslash, "info sources" shows the same file twice, like this:

  (gdb) info sources
  Source files for which symbols have been read in:



  Source files for which symbols will be read in on demand:

  d:/usr/eli/data/dbw.c, d:\usr\eli/data\dbw.c

This is with GDB 7.2 and GCC 3.4.2.  That means we compare files with
strcmp/strcasecmp somewhere, and don't know that / and \ are
equivalent here.  Or maybe it's a bug in the ancient version of GCC I
use.

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

2011/3/9 Eli Zaretskii <eliz@gnu.org>:
>> From: Pedro Alves <pedro@codesourcery.com>
>> Date: Wed, 9 Mar 2011 12:58:38 +0000
>> Cc: gdb-patches@sourceware.org,
>>  dj@redhat.com,
>>  ktietz70@googlemail.com,
>>  binutils@sourceware.org,
>>  gcc-patches@gcc.gnu.org
>>
>> > > The one's left are: 1 in a linux-native only file (never cares
>> > > for other filesystem semantics), and a couple in the coff and
>> > > mdebug readers.  The latter could be rewritten in terms of
>> > > lbasename, but I'm not sure whether gcc outputs a literal '/' in
>> > > that case even when building on mingw.  If so, and we changed them,
>> > > we'd be breaking reading these files on Windows
>> >
>> > Sorry, I don't understand how would that break on Windows.  Could you
>> > elaborate?  And what "couple of coff and mdebug readers" did you have
>> > in mind?
>>
>> Sorry, in the hurry, I had a (another) brain cramp.  Wouldn't break.
>> Still it'd be useless to change this _if_ gcc hardcodes '/'.  Dunno
>> whether it does.
>
> At least on MinGW, GCC simply uses whatever was passed on the command
> line.  I tested that by compiling the same source file, passing it to
> GCC with different flavors of slashes, including mixed ones.  Then in
> GDB I typed "info sources" and saw the source file with exactly the
> same flavor of slashes as what I typed on the GCC command line.
>
> Funnily enough, when the file name given to GCC includes at least one
> backslash, "info sources" shows the same file twice, like this:
>
>  (gdb) info sources
>  Source files for which symbols have been read in:
>
>
>
>  Source files for which symbols will be read in on demand:
>
>  d:/usr/eli/data/dbw.c, d:\usr\eli/data\dbw.c
>
> This is with GDB 7.2 and GCC 3.4.2.  That means we compare files with
> strcmp/strcasecmp somewhere, and don't know that / and \ are
> equivalent here.  Or maybe it's a bug in the ancient version of GCC I
> use.

Yes, this observation is related to some comparision tweaks in libcpp
and in some other parts in gcc about filenames. When gcc gets into
stage 1, I will post the prepared patch for this.

Kai

Build regression [Re: [patch libiberty include]: Add additional helper functions for directory-separator searching]

[Jan Kratochvil]

On Wed, 09 Mar 2011 13:58:38 +0100, Pedro Alves wrote:
> Thanks.  I've applied it.

nto-tdep.c:130:8: error: assignment discards ‘const’ qualifier from pointer target type [-Werror]

gcc-4.6.0-0.12.fc15.x86_64
--with-system-zlib --enable-64-bit-bfd --enable-targets=all --enable-static --disable-shared --enable-debug --disable-sim --enable-gold --with-separate-debug-dir=/usr/lib/debug CC=gcc CFLAGS=-m64 -ggdb2 -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 LDFLAGS= -lmcheck

commit 7403e6b3f0f7d4c4f80703486f602ee5e2c9a3dd
Author: Pedro Alves <pedro@codesourcery.com>
Date:   Wed Mar 9 12:48:53 2011 +0000

        * cli/cli-cmds.c (shell_escape): Use lbasename.
        * coffread.c (coff_start_symtab): Constify parameter.
        (complete_symtab): Constify `name' parameter.
        (coff_symtab_read): Constify `filestring' local.
        (coff_getfilename): Constify return and `result' local.
        Use lbasename.
        * fbsd-nat.c (fbsd_make_corefile_notes): Use lbasename.
        * linux-fork.c (info_checkpoints_command): Use lbasename.
        * linux-nat.c (linux_nat_make_corefile_notes): Use lbasename.
        * minsyms.c (lookup_minimal_symbol): Use lbasename.
        * nto-tdep.c (nto_find_and_open_solib): Use lbasename.
        * procfs.c (procfs_make_note_section): Use lbasename.
        * tui/tui-io.c (printable_part): Constity return and parameter.
        Use lbasename.
        (print_filename): Constify parameters, and local `s'.
        (tui_rl_display_match_list): Constify local `temp'.


Thanks,
Jan

Re: Build regression [Re: [patch libiberty include]: Add additional helper functions for directory-separator searching]

[Pedro Alves]

On Wednesday 09 March 2011 14:37:15, Jan Kratochvil wrote:
> On Wed, 09 Mar 2011 13:58:38 +0100, Pedro Alves wrote:
> > Thanks.  I've applied it.
> 
> nto-tdep.c:130:8: error: assignment discards ‘const’ qualifier from pointer target type [-Werror]

Thanks, had forgotten --enable-targets=all.  Sorry about that.

Applied.

-- 
Pedro Alves

2011-03-09  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* nto-tdep.c (nto_find_and_open_solib): Constify local `base'.

---
 gdb/nto-tdep.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: src/gdb/nto-tdep.c
===================================================================
--- src.orig/gdb/nto-tdep.c	2011-03-09 12:47:34.000000000 +0000
+++ src/gdb/nto-tdep.c	2011-03-09 14:56:44.182800000 +0000
@@ -89,7 +89,8 @@ nto_map_arch_to_cputype (const char *arc
 int
 nto_find_and_open_solib (char *solib, unsigned o_flags, char **temp_pathname)
 {
-  char *buf, *arch_path, *nto_root, *endian, *base;
+  char *buf, *arch_path, *nto_root, *endian;
+  const char *base;
   const char *arch;
   int ret;
 #define PATH_FMT \

Re: Build regression [Re: [patch libiberty include]: Add additional helper functions for directory-separator searching]

[Jan Kratochvil]

On Wed, 09 Mar 2011 16:02:36 +0100, Pedro Alves wrote:
> Thanks, had forgotten --enable-targets=all.  Sorry about that.

It builds now OK.


Thanks,
Jan

Re: [patch libiberty include]: Add additional helper functions for directory-separator searching

[Kai Tietz]

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

2011/3/8 DJ Delorie <dj@redhat.com>:
>
>> I think we don't need filename_dirchr, only filename_dirrchr.
>
> I see no harm in having both, for completeness, though.  One could
> argue they should be in separate files, but they're trivial functions
> on non-dos-fs systems.
>
> What bothers me about this patch is that we're adding yet another set
> of functions that don't discriminate between the host filesystem, the
> target filesystem, and the build filesystem.
>

Ok, I got the point about having here implementations which can handle
both cases. So I added an additional argument to those functions to
specify for which file-system flavor is searched.
I don't see right now much use for this difference, but well, it won't
harm to have this ability too.

2011-03-12  Kai Tietz

	* filename.h (filename_dirchr_host): New macro.
	(filename_dirrchr_host): New macro.
	(filename_dirchr): New prototype.
	(filename_dirrchr): New prototype.

2011-03-12  Kai Tietz

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

Tested for x86_64-w64-mingw32, and x86_64-pc-linux-gnu. Ok for apply?

Regards,
Kai

[-- Attachment #2: libiberty_dirsep.txt --]
[-- Type: text/plain, Size: 7559 bytes --]

Index: gcc/include/filenames.h
===================================================================
--- gcc.orig/include/filenames.h	2011-03-12 17:02:03.802364200 +0100
+++ gcc/include/filenames.h	2011-03-12 17:31:48.887339800 +0100
@@ -37,10 +37,14 @@ extern "C" {
 #  define HAS_DRIVE_SPEC(f) HAS_DOS_DRIVE_SPEC (f)
 #  define IS_DIR_SEPARATOR(c) IS_DOS_DIR_SEPARATOR (c)
 #  define IS_ABSOLUTE_PATH(f) IS_DOS_ABSOLUTE_PATH (f)
+#  define filename_dirchr_host(P) filename_dirchr (1, (P))
+#  define filename_dirrchr_host(P) filename_dirrchr (1, (P))
 #else /* not DOSish */
 #  define HAS_DRIVE_SPEC(f) (0)
 #  define IS_DIR_SEPARATOR(c) IS_UNIX_DIR_SEPARATOR (c)
 #  define IS_ABSOLUTE_PATH(f) IS_UNIX_ABSOLUTE_PATH (f)
+#  define filename_dirchr_host(P) filename_dirchr (0, (P))
+#  define filename_dirrchr_host(P) filename_dirrchr (0, (P))
 #endif
 
 #define IS_DIR_SEPARATOR_1(dos_based, c)				\
@@ -76,6 +80,9 @@ extern int filename_cmp (const char *s1,
 extern int filename_ncmp (const char *s1, const char *s2,
 			  size_t n);
 
+extern char *filename_dirchr (int dos_based, const char *p);
+extern char *filename_dirrchr (int dos_based, const char *p);
+
 #ifdef __cplusplus
 }
 #endif
Index: gcc/libiberty/functions.texi
===================================================================
--- gcc.orig/libiberty/functions.texi	2011-03-12 17:02:03.817364200 +0100
+++ gcc/libiberty/functions.texi	2011-03-12 17:33:02.731563500 +0100
@@ -296,6 +296,32 @@ and backward slashes are equal.
 
 @end deftypefn
 
+@c filename_chr.c:32
+@deftypefn Extension int filename_dirchr (int @var{dos_based}, const char *@var{p})
+
+This function searchs for first occurance of a directory-separator character in
+the filename @var{p} from left to right direction.
+If argument @var{dos_based} is not zero, in addition to the UNIX-style slash
+also the DOS-style backslash is searched as directory separator.
+This function does not normalize file name.  The result of this routine is
+@code{NULL} pointer, if no match was found. Otherwise it returns the pointer
+to the found match.
+
+@end deftypefn
+
+@c filename_chr.c:65
+@deftypefn Extension int filename_dirrchr (int @var{dos_based}, const char *@var{p})
+
+This function searchs for first occurance of a directory-separator character in
+the filename @var{p} from right to left direction.
+If argument @var{dos_based} is not zero, in addition to the UNIX-style slash
+also the DOS-style backslash is treated as directory separator.
+This function does not normalize file name.  The result of this routine is
+@code{NULL} pointer, if no match was found. Otherwise it returns the pointer
+to the found match.
+
+@end deftypefn
+
 @c filename_cmp.c:81
 @deftypefn Extension int filename_ncmp (const char *@var{s1}, const char *@var{s2}, size_t @var{n})
 
Index: gcc/libiberty/Makefile.in
===================================================================
--- gcc.orig/libiberty/Makefile.in	2011-03-12 17:02:03.829364200 +0100
+++ gcc/libiberty/Makefile.in	2011-03-12 17:06:49.519580900 +0100
@@ -127,8 +127,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 crc32.c				\
 	dyn-string.c							\
-	fdmatch.c ffs.c fibheap.c filename_cmp.c floatformat.c		\
-	fnmatch.c fopen_unlocked.c					\
+	fdmatch.c ffs.c fibheap.c filename_chr.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							\
@@ -168,7 +168,8 @@ REQUIRED_OFILES =							\
 	./choose-temp.$(objext) ./concat.$(objext)			\
 	./cp-demint.$(objext) ./crc32.$(objext) ./dyn-string.$(objext)	\
 	./fdmatch.$(objext) ./fibheap.$(objext)				\
-	./filename_cmp.$(objext) ./floatformat.$(objext)		\
+	./filename_chr.$(objext) ./filename_cmp.$(objext)		\
+	./floatformat.$(objext)						\
 	./fnmatch.$(objext) ./fopen_unlocked.$(objext)			\
 	./getopt.$(objext) ./getopt1.$(objext) ./getpwd.$(objext)	\
 	./getruntime.$(objext) ./hashtab.$(objext) ./hex.$(objext)	\
@@ -646,6 +647,13 @@ $(CONFIGURED_OFILES): stamp-picdir
 	else true; fi
 	$(COMPILE.c) $(srcdir)/fibheap.c $(OUTPUT_OPTION)
 
+./filename_chr.$(objext): $(srcdir)/filename_chr.c config.h $(INCDIR)/filenames.h \
+	$(INCDIR)/safe-ctype.h
+	if [ x"$(PICFLAG)" != x ]; then \
+	  $(COMPILE.c) $(PICFLAG) $(srcdir)/filename_chr.c -o pic/$@; \
+	else true; fi
+	$(COMPILE.c) $(srcdir)/filename_chr.c $(OUTPUT_OPTION)
+
 ./filename_cmp.$(objext): $(srcdir)/filename_cmp.c config.h $(INCDIR)/filenames.h \
 	$(INCDIR)/safe-ctype.h
 	if [ x"$(PICFLAG)" != x ]; then \
Index: gcc/libiberty/filename_chr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/libiberty/filename_chr.c	2011-03-12 17:31:11.640209400 +0100
@@ -0,0 +1,96 @@
+/* File name character searching routines.
+
+   Copyright (C) 2011 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_CONFIG_H
+#include "config.h"
+#endif
+
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
+
+#include "filenames.h"
+#include "safe-ctype.h"
+
+/*
+
+@deftypefn Extension int filename_dirchr (int @var{dos_based}, const char *@var{p})
+
+This function searchs for first occurance of a directory-separator character in
+the filename @var{p} from left to right direction.
+If argument @var{dos_based} is not zero, in addition to the UNIX-style slash
+also the DOS-style backslash is searched as directory separator.
+This function does not normalize file name.  The result of this routine is
+@code{NULL} pointer, if no match was found. Otherwise it returns the pointer
+to the found match.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirchr (int dos_based, const char *p)
+{
+  char *r;
+
+  if (!p)
+    return NULL;
+  r = strchr (p, '/');
+  if (dos_based)
+    {
+      char *r2 = strchr (p, '\\');
+      if (!r || (r2 && r2 < r))
+	r = r2;
+    }
+  return r;
+}
+
+/*
+
+@deftypefn Extension int filename_dirrchr (int @var{dos_based}, const char *@var{p})
+
+This function searchs for first occurance of a directory-separator character in
+the filename @var{p} from right to left direction.
+If argument @var{dos_based} is not zero, in addition to the UNIX-style slash
+also the DOS-style backslash is treated as directory separator.
+This function does not normalize file name.  The result of this routine is
+@code{NULL} pointer, if no match was found. Otherwise it returns the pointer
+to the found match.
+
+@end deftypefn
+
+*/
+
+char *
+filename_dirrchr (int dos_based, const char *p)
+{
+  char *r;
+
+  if (!p)
+    return NULL;
+
+  r = strrchr (p, '/');
+  if (dos_based)
+    {
+      char *r2 = strrchr (p, '\\');
+
+      if (!r || (r2 && r2 > r))
+	r = r2;
+    }
+  return r;
+}