This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdb: Make ldirname return a std::string
- From: Philipp Rudo <prudo at linux dot vnet dot ibm dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 22 Mar 2017 19:01:35 +0100
- Subject: Re: [PATCH] gdb: Make ldirname return a std::string
- Authentication-results: sourceware.org; auth=none
- References: <1490198646-10469-1-git-send-email-palves@redhat.com>
Hi
On Wed, 22 Mar 2017 16:04:06 +0000
Pedro Alves <palves@redhat.com> wrote:
[...]
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index b3ea52b..4233006 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
[...]
> @@ -9122,37 +9130,37 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu)
> return cu->producer_is_gcc_lt_4_3;
> }
>
> -static void
> -find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu,
> - const char **name, const char **comp_dir)
> +static file_and_directory
> +find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
> {
> + file_and_directory res;
> +
> /* Find the filename. Do not use dwarf2_name here, since the
> filename is not a source language identifier. */
> - *name = dwarf2_string_attr (die, DW_AT_name, cu);
> - *comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
> + res.name = dwarf2_string_attr (die, DW_AT_name, cu);
> + res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
>
> - if (*comp_dir == NULL
> - && producer_is_gcc_lt_4_3 (cu) && *name != NULL
> - && IS_ABSOLUTE_PATH (*name))
> + if (res.comp_dir == NULL
> + && producer_is_gcc_lt_4_3 (cu) && res.name != NULL
> + && IS_ABSOLUTE_PATH (res.name))
> {
> - char *d = ldirname (*name);
> -
> - *comp_dir = d;
> - if (d != NULL)
> - make_cleanup (xfree, d);
> + res.comp_dir_storage = ldirname (res.name);
> + res.comp_dir = res.comp_dir_storage.c_str ();
> }
> - if (*comp_dir != NULL)
> + if (res.comp_dir != NULL)
Is this check valid? Doesn't std::string.c_str () always return a
pointer != NULL?
I would check for res.comp_dir_storage.empty ().
Otherwise the patch is ok.
Philipp
> {
> /* Irix 6.2 native cc prepends <machine>.: to the compilation
> directory, get rid of it. */
> - const char *cp = strchr (*comp_dir, ':');
> + const char *cp = strchr (res.comp_dir, ':');
>
> - if (cp && cp != *comp_dir && cp[-1] == '.' && cp[1] == '/')
> - *comp_dir = cp + 1;
> + if (cp && cp != res.comp_dir && cp[-1] == '.' && cp[1] == '/')
> + res.comp_dir = cp + 1;
> }
>
> - if (*name == NULL)
> - *name = "<unknown>";
> + if (res.name == NULL)
> + res.name = "<unknown>";
> +
> + return res;
> }
>
> /* Handle DW_AT_stmt_list for a compilation unit.
> @@ -9262,12 +9270,9 @@ read_file_scope (struct die_info *die, struct
> dwarf2_cu *cu) {
> struct objfile *objfile = dwarf2_per_objfile->objfile;
> struct gdbarch *gdbarch = get_objfile_arch (objfile);
> - struct cleanup *back_to = make_cleanup (null_cleanup, 0);
> CORE_ADDR lowpc = ((CORE_ADDR) -1);
> CORE_ADDR highpc = ((CORE_ADDR) 0);
> struct attribute *attr;
> - const char *name = NULL;
> - const char *comp_dir = NULL;
> struct die_info *child_die;
> CORE_ADDR baseaddr;
>
> @@ -9281,7 +9286,7 @@ read_file_scope (struct die_info *die, struct
> dwarf2_cu *cu) lowpc = highpc;
> lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr);
>
> - find_file_and_directory (die, cu, &name, &comp_dir);
> + file_and_directory fnd = find_file_and_directory (die, cu);
>
> prepare_one_comp_unit (cu, die, cu->language);
>
> @@ -9295,12 +9300,12 @@ read_file_scope (struct die_info *die, struct
> dwarf2_cu *cu) if (cu->producer && strstr (cu->producer, "GNU Go
> ") != NULL) set_cu_language (DW_LANG_Go, cu);
>
> - dwarf2_start_symtab (cu, name, comp_dir, lowpc);
> + dwarf2_start_symtab (cu, fnd.name, fnd.comp_dir, lowpc);
>
> /* Decode line number information if present. We do this before
> processing child DIEs, so that the line header table is
> available for DW_AT_decl_file. */
> - handle_DW_AT_stmt_list (die, cu, comp_dir, lowpc);
> + handle_DW_AT_stmt_list (die, cu, fnd.comp_dir, lowpc);
>
> /* Process all dies in compilation unit. */
> if (die->child != NULL)
> @@ -9338,8 +9343,6 @@ read_file_scope (struct die_info *die, struct
> dwarf2_cu *cu) dwarf_decode_macros (cu, macro_offset, 0);
> }
> }
> -
> - do_cleanups (back_to);
> }
>
> /* TU version of handle_DW_AT_stmt_list for read_type_unit_scope.
> @@ -10892,49 +10895,44 @@ open_and_init_dwp_file (void)
> {
> struct objfile *objfile = dwarf2_per_objfile->objfile;
> struct dwp_file *dwp_file;
> - char *dwp_name;
> - struct cleanup *cleanups = make_cleanup (null_cleanup, 0);
>
> /* Try to find first .dwp for the binary file before any symbolic
> links resolving. */
>
> /* If the objfile is a debug file, find the name of the real binary
> file and get the name of dwp file from there. */
> + std::string dwp_name;
> if (objfile->separate_debug_objfile_backlink != NULL)
> {
> struct objfile *backlink =
> objfile->separate_debug_objfile_backlink; const char
> *backlink_basename = lbasename (backlink->original_name);
> - char *debug_dirname = ldirname (objfile->original_name);
>
> - make_cleanup (xfree, debug_dirname);
> - dwp_name = xstrprintf ("%s%s%s.dwp", debug_dirname,
> - SLASH_STRING, backlink_basename);
> + dwp_name = ldirname (objfile->original_name) + SLASH_STRING +
> backlink_basename; }
> else
> - dwp_name = xstrprintf ("%s.dwp", objfile->original_name);
> - make_cleanup (xfree, dwp_name);
> + dwp_name = objfile->original_name;
> +
> + dwp_name += ".dwp";
>
> - gdb_bfd_ref_ptr dbfd (open_dwp_file (dwp_name));
> + gdb_bfd_ref_ptr dbfd (open_dwp_file (dwp_name.c_str ()));
> if (dbfd == NULL
> && strcmp (objfile->original_name, objfile_name (objfile)) !=
> 0) {
> /* Try to find .dwp for the binary file after gdb_realpath
> resolving. */
> - dwp_name = xstrprintf ("%s.dwp", objfile_name (objfile));
> - make_cleanup (xfree, dwp_name);
> - dbfd = open_dwp_file (dwp_name);
> + dwp_name = objfile_name (objfile);
> + dwp_name += ".dwp";
> + dbfd = open_dwp_file (dwp_name.c_str ());
> }
>
> if (dbfd == NULL)
> {
> if (dwarf_read_debug)
> - fprintf_unfiltered (gdb_stdlog, "DWP file not found: %s\n",
> dwp_name);
> - do_cleanups (cleanups);
> + fprintf_unfiltered (gdb_stdlog, "DWP file not found: %s\n",
> dwp_name.c_str ()); return NULL;
> }
> dwp_file = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct
> dwp_file); dwp_file->name = bfd_get_filename (dbfd.get ());
> dwp_file->dbfd = dbfd.release ();
> - do_cleanups (cleanups);
>
> /* +1: section 0 is unused */
> dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
> @@ -10958,7 +10956,7 @@ open_and_init_dwp_file (void)
> error (_("Dwarf Error: DWP file CU version %s doesn't match"
> " TU version %s [in DWP file %s]"),
> pulongest (dwp_file->cus->version),
> - pulongest (dwp_file->tus->version), dwp_name);
> + pulongest (dwp_file->tus->version), dwp_name.c_str ());
> }
> dwp_file->version = dwp_file->cus->version;
>
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 73fb3d0..a7aff53 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1550,7 +1550,7 @@ do_start_initialization ()
> /foo/bin/python
> /foo/lib/pythonX.Y/...
> This must be done before calling Py_Initialize. */
> - progname = concat (ldirname (python_libdir), SLASH_STRING, "bin",
> + progname = concat (ldirname (python_libdir).c_str (),
> SLASH_STRING, "bin", SLASH_STRING, "python", (char *) NULL);
> #ifdef IS_PY3K
> oldloc = xstrdup (setlocale (LC_ALL, NULL));
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 27021a1..39798cc 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -2943,20 +2943,19 @@ dummy_obstack_deallocate (void *object, void
> *data) /* Simple, portable version of dirname that does not modify its
> argument. */
>
> -char *
> +std::string
> ldirname (const char *filename)
> {
> + std::string dirname;
> const char *base = lbasename (filename);
> - char *dirname;
>
> while (base > filename && IS_DIR_SEPARATOR (base[-1]))
> --base;
>
> if (base == filename)
> - return NULL;
> + return dirname;
>
> - dirname = (char *) xmalloc (base - filename + 2);
> - memcpy (dirname, filename, base - filename);
> + dirname = std::string (filename, base - filename);
>
> /* On DOS based file systems, convert "d:foo" to "d:.", so that we
> create "d:./bar" later instead of the (different) "d:/bar". */
> @@ -2964,7 +2963,6 @@ ldirname (const char *filename)
> && !IS_DIR_SEPARATOR (filename[0]))
> dirname[base++ - filename] = '.';
>
> - dirname[base - filename] = '\0';
> return dirname;
> }
>
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f138702..fb75f2e 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -135,7 +135,7 @@ extern int gdb_filename_fnmatch (const char
> *pattern, const char *string, extern void substitute_path_component
> (char **stringp, const char *from, const char *to);
>
> -char *ldirname (const char *filename);
> +std::string ldirname (const char *filename);
>
> extern int count_path_elements (const char *path);
>
> diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
> index 1e42b8d..a436418 100644
> --- a/gdb/xml-syscall.c
> +++ b/gdb/xml-syscall.c
> @@ -363,7 +363,6 @@ static struct syscalls_info *
> xml_init_syscalls_info (const char *filename)
> {
> char *full_file;
> - char *dirname;
> struct syscalls_info *syscalls_info;
> struct cleanup *back_to;
>
> @@ -373,12 +372,9 @@ xml_init_syscalls_info (const char *filename)
>
> back_to = make_cleanup (xfree, full_file);
>
> - dirname = ldirname (filename);
> - if (dirname != NULL)
> - make_cleanup (xfree, dirname);
> -
> syscalls_info = syscall_parse_xml (full_file,
> - xml_fetch_content_from_file,
> dirname);
> + xml_fetch_content_from_file,
> + (void *) ldirname
> (filename).c_str ()); do_cleanups (back_to);
>
> return syscalls_info;
> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 1677659..effe652 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -694,7 +694,6 @@ file_read_description_xml (const char *filename)
> struct target_desc *tdesc;
> char *tdesc_str;
> struct cleanup *back_to;
> - char *dirname;
>
> tdesc_str = xml_fetch_content_from_file (filename, NULL);
> if (tdesc_str == NULL)
> @@ -705,11 +704,8 @@ file_read_description_xml (const char *filename)
>
> back_to = make_cleanup (xfree, tdesc_str);
>
> - dirname = ldirname (filename);
> - if (dirname != NULL)
> - make_cleanup (xfree, dirname);
> -
> - tdesc = tdesc_parse_xml (tdesc_str, xml_fetch_content_from_file,
> dirname);
> + tdesc = tdesc_parse_xml (tdesc_str, xml_fetch_content_from_file,
> + (void *) ldirname (filename).c_str ());
> do_cleanups (back_to);
>
> return tdesc;