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

[Pedro Alves <pedro@codesourcery.com>]

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;