From patchwork Wed Mar 9 11:46:36 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 86102 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id C3696B70E1 for ; Wed, 9 Mar 2011 22:46:53 +1100 (EST) Received: (qmail 31975 invoked by alias); 9 Mar 2011 11:46:47 -0000 Received: (qmail 31959 invoked by uid 22791); 9 Mar 2011 11:46:45 -0000 X-SWARE-Spam-Status: No, hits=0.9 required=5.0 tests=AWL, BAYES_00, KAM_STOCKTIP, TW_CP, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 09 Mar 2011 11:46:36 +0000 Received: (qmail 7308 invoked from network); 9 Mar 2011 11:46:34 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 9 Mar 2011 11:46:34 -0000 From: Pedro Alves To: Eli Zaretskii Subject: Re: [patch libiberty include]: Add additional helper functions for directory-separator searching Date: Wed, 9 Mar 2011 11:46:36 +0000 User-Agent: KMail/1.13.5 (Linux/2.6.35-27-generic; KDE/4.6.1; x86_64; ; ) Cc: gdb-patches@sourceware.org, dj@redhat.com, ktietz70@googlemail.com, binutils@sourceware.org, gcc-patches@gcc.gnu.org References: <201103082238.00289.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Message-Id: <201103091146.36746.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list 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. 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. 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;