Message ID | 201103091146.36746.pedro@codesourcery.com |
---|---|
State | New |
Headers | show |
> 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
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); } 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;