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

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.

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

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;		
	      }

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

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

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