* [PATCH] gdb: Make ldirname return a std::string
@ 2017-03-22 16:04 Pedro Alves
2017-03-22 18:01 ` Philipp Rudo
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2017-03-22 16:04 UTC (permalink / raw)
To: gdb-patches
Eliminates several uses of cleanups.
(And if applied before
<https://sourceware.org/ml/gdb-patches/2017-03/msg00394.html>, fixes
the leak in python.c:do_start_initialization too.)
Tested on x86_64 Fedora 23 with Python 2 and 3.
gdb/ChangeLog
2017-03-22 Pedro Alves <palves@redhat.com>
* dwarf2read.c (struct file_and_directory): New.
(dwarf2_get_dwz_file): Adjust to use std::string.
(dw2_get_file_names_reader): Adjust to use file_and_directory.
(find_file_and_directory): Adjust to return a file_and_directory
object.
(read_file_scope): Adjust to use file_and_directory. Remove
make_cleanup/do_cleanups calls.
(open_and_init_dwp_file): Adjust to use std::string. Remove
make_cleanup/do_cleanups calls.
* python/python.c (do_start_initialization): Adjust to ldirname
returning a std::string.
* utils.c (ldirname): Now returns a std::string.
* utils.h (ldirname): Change return type to std::string.
* xml-syscall.c (xml_init_syscalls_info): Adjust to ldirname
returning a std::string.
* xml-tdesc.c (file_read_description_xml): Likewise.
---
gdb/dwarf2read.c | 108 ++++++++++++++++++++++++++--------------------------
gdb/python/python.c | 2 +-
gdb/utils.c | 10 ++---
gdb/utils.h | 2 +-
gdb/xml-syscall.c | 8 +---
gdb/xml-tdesc.c | 8 +---
6 files changed, 63 insertions(+), 75 deletions(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b3ea52b..4233006 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1885,9 +1885,20 @@ static void queue_comp_unit (struct dwarf2_per_cu_data *per_cu,
static void process_queue (void);
-static void find_file_and_directory (struct die_info *die,
- struct dwarf2_cu *cu,
- const char **name, const char **comp_dir);
+/* The return type of find_file_and_directory. */
+
+struct file_and_directory
+{
+ const char *name;
+ const char *comp_dir;
+
+ /* If we needed to build a new string for comp_dir, this is what
+ owns the storage. */
+ std::string comp_dir_storage;
+};
+
+static file_and_directory find_file_and_directory (struct die_info *die,
+ struct dwarf2_cu *cu);
static char *file_full_name (int file, struct line_header *lh,
const char *comp_dir);
@@ -2552,18 +2563,15 @@ dwarf2_get_dwz_file (void)
buildid_len = (size_t) buildid_len_arg;
filename = (const char *) data;
+
+ std::string abs_storage;
if (!IS_ABSOLUTE_PATH (filename))
{
char *abs = gdb_realpath (objfile_name (dwarf2_per_objfile->objfile));
- char *rel;
make_cleanup (xfree, abs);
- abs = ldirname (abs);
- make_cleanup (xfree, abs);
-
- rel = concat (abs, SLASH_STRING, filename, (char *) NULL);
- make_cleanup (xfree, rel);
- filename = rel;
+ abs_storage = ldirname (abs) + SLASH_STRING + filename;
+ filename = abs_storage.c_str ();
}
/* First try the file name given in the section. If that doesn't
@@ -3385,13 +3393,13 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
gdb_assert (slot != NULL);
*slot = qfn;
- find_file_and_directory (comp_unit_die, cu, &name, &comp_dir);
+ file_and_directory fnd = find_file_and_directory (comp_unit_die, cu);
qfn->num_file_names = lh->num_file_names;
qfn->file_names =
XOBNEWVEC (&objfile->objfile_obstack, const char *, lh->num_file_names);
for (i = 0; i < lh->num_file_names; ++i)
- qfn->file_names[i] = file_full_name (i + 1, lh, comp_dir);
+ qfn->file_names[i] = file_full_name (i + 1, lh, fnd.comp_dir);
qfn->real_names = NULL;
free_line_header (lh);
@@ -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)
{
/* 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;
--
2.5.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb: Make ldirname return a std::string
2017-03-22 16:04 [PATCH] gdb: Make ldirname return a std::string Pedro Alves
@ 2017-03-22 18:01 ` Philipp Rudo
2017-03-22 19:26 ` [PATCH v2] " Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Philipp Rudo @ 2017-03-22 18:01 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
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;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] gdb: Make ldirname return a std::string
2017-03-22 18:01 ` Philipp Rudo
@ 2017-03-22 19:26 ` Pedro Alves
2017-03-23 10:04 ` Philipp Rudo
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2017-03-22 19:26 UTC (permalink / raw)
To: Philipp Rudo; +Cc: gdb-patches
On 03/22/2017 06:01 PM, Philipp Rudo wrote:
>> - 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?
Yes, std::string.c_str () always return a non-null pointer, but
if the condition in the if above:
if (res.comp_dir == NULL
&& producer_is_gcc_lt_4_3 (cu) && res.name != NULL
&& IS_ABSOLUTE_PATH (res.name))
is false, and further above, this:
res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
... had left res.comp_dir NULL too, then we get here with
res.comp_dir still NULL.
> I would check for res.comp_dir_storage.empty ().
That won't work. The "comp_dir_storage" field is there
for the case when we needed to call ldirname.
That's what replaces the make_cleanup call. I.e., before, the memory
for the result of that ldirname call was released whenever
something called do_cleanups afterwards. While after this
patch, that memory is owned by file_and_directory, via the
comp_dir_storage field. But that storage is _not_ used if the
res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
line above managed to find a comp_dir attribute. In that
case, res.comp_dir points to some memory owned by the obstack
that owns DIE/CU, and the storage field is left empty. I see I
forgot to add some comments. Fixed in this update below.
I shouldn't point res.comp_dir to the storage if ldirname returned an
empty string though. Fixed now too.
From 98a4e4d213eb85b264c50bcae7664f5fb3cd29d7 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 22 Mar 2017 15:21:29 +0000
Subject: [PATCH v2] gdb: Make ldirname return a std::string
Eliminates several uses of cleanups.
(And if applied before
<https://sourceware.org/ml/gdb-patches/2017-03/msg00394.html>, fixes
the leak in python.c:do_start_initialization too.)
Tested on x86_64 Fedora 23 with Python 2 and 3.
gdb/ChangeLog
2017-03-22 Pedro Alves <palves@redhat.com>
* dwarf2read.c (struct file_and_directory): New.
(dwarf2_get_dwz_file): Adjust to use std::string.
(dw2_get_file_names_reader): Adjust to use file_and_directory.
(find_file_and_directory): Adjust to return a file_and_directory
object.
(read_file_scope): Adjust to use file_and_directory. Remove
make_cleanup/do_cleanups calls.
(open_and_init_dwp_file): Adjust to use std::string. Remove
make_cleanup/do_cleanups calls.
* python/python.c (do_start_initialization): Adjust to ldirname
returning a std::string.
* utils.c (ldirname): Now returns a std::string.
* utils.h (ldirname): Change return type to std::string.
* xml-syscall.c (xml_init_syscalls_info): Adjust to ldirname
returning a std::string.
* xml-tdesc.c (file_read_description_xml): Likewise.
---
gdb/dwarf2read.c | 117 +++++++++++++++++++++++++++-------------------------
gdb/python/python.c | 2 +-
gdb/utils.c | 10 ++---
gdb/utils.h | 2 +-
gdb/xml-syscall.c | 8 +---
gdb/xml-tdesc.c | 8 +---
6 files changed, 71 insertions(+), 76 deletions(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b3ea52b..36b6ead 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1885,9 +1885,27 @@ static void queue_comp_unit (struct dwarf2_per_cu_data *per_cu,
static void process_queue (void);
-static void find_file_and_directory (struct die_info *die,
- struct dwarf2_cu *cu,
- const char **name, const char **comp_dir);
+/* The return type of find_file_and_directory. Note, the enclosed
+ string pointers are only valid while this object is valid. */
+
+struct file_and_directory
+{
+ /* The filename. This is never NULL. */
+ const char *name;
+
+ /* The compilation directory. NULL if not known. If we needed to
+ compute a new string, this points to COMP_DIR_STORAGE, otherwise,
+ points directly to the DW_AT_comp_dir string attribute owned by
+ the obstack that owns the DIE. */
+ const char *comp_dir;
+
+ /* If we needed to build a new string for comp_dir, this is what
+ owns the storage. */
+ std::string comp_dir_storage;
+};
+
+static file_and_directory find_file_and_directory (struct die_info *die,
+ struct dwarf2_cu *cu);
static char *file_full_name (int file, struct line_header *lh,
const char *comp_dir);
@@ -2552,18 +2570,15 @@ dwarf2_get_dwz_file (void)
buildid_len = (size_t) buildid_len_arg;
filename = (const char *) data;
+
+ std::string abs_storage;
if (!IS_ABSOLUTE_PATH (filename))
{
char *abs = gdb_realpath (objfile_name (dwarf2_per_objfile->objfile));
- char *rel;
make_cleanup (xfree, abs);
- abs = ldirname (abs);
- make_cleanup (xfree, abs);
-
- rel = concat (abs, SLASH_STRING, filename, (char *) NULL);
- make_cleanup (xfree, rel);
- filename = rel;
+ abs_storage = ldirname (abs) + SLASH_STRING + filename;
+ filename = abs_storage.c_str ();
}
/* First try the file name given in the section. If that doesn't
@@ -3332,7 +3347,6 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
struct line_header *lh;
struct attribute *attr;
int i;
- const char *name, *comp_dir;
void **slot;
struct quick_file_names *qfn;
unsigned int line_offset;
@@ -3385,13 +3399,13 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
gdb_assert (slot != NULL);
*slot = qfn;
- find_file_and_directory (comp_unit_die, cu, &name, &comp_dir);
+ file_and_directory fnd = find_file_and_directory (comp_unit_die, cu);
qfn->num_file_names = lh->num_file_names;
qfn->file_names =
XOBNEWVEC (&objfile->objfile_obstack, const char *, lh->num_file_names);
for (i = 0; i < lh->num_file_names; ++i)
- qfn->file_names[i] = file_full_name (i + 1, lh, comp_dir);
+ qfn->file_names[i] = file_full_name (i + 1, lh, fnd.comp_dir);
qfn->real_names = NULL;
free_line_header (lh);
@@ -9122,37 +9136,38 @@ 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);
+ if (!res.comp_dir_storage.empty ())
+ res.comp_dir = res.comp_dir_storage.c_str ();
}
- if (*comp_dir != NULL)
+ if (res.comp_dir != NULL)
{
/* 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 +9277,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 +9293,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 +9307,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 +9350,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 +10902,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 +10963,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;
--
2.5.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] gdb: Make ldirname return a std::string
2017-03-22 19:26 ` [PATCH v2] " Pedro Alves
@ 2017-03-23 10:04 ` Philipp Rudo
2017-03-27 11:12 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Philipp Rudo @ 2017-03-23 10:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Wed, 22 Mar 2017 19:26:40 +0000
Pedro Alves <palves@redhat.com> wrote:
> On 03/22/2017 06:01 PM, Philipp Rudo wrote:
>
> >> - 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?
>
> Yes, std::string.c_str () always return a non-null pointer, but
> if the condition in the if above:
>
> if (res.comp_dir == NULL
> && producer_is_gcc_lt_4_3 (cu) && res.name != NULL
> && IS_ABSOLUTE_PATH (res.name))
>
> is false, and further above, this:
>
> res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
>
> ... had left res.comp_dir NULL too, then we get here with
> res.comp_dir still NULL.
You are right.
> > I would check for res.comp_dir_storage.empty ().
>
> That won't work. The "comp_dir_storage" field is there
> for the case when we needed to call ldirname.
> That's what replaces the make_cleanup call. I.e., before, the memory
> for the result of that ldirname call was released whenever
> something called do_cleanups afterwards. While after this
> patch, that memory is owned by file_and_directory, via the
> comp_dir_storage field. But that storage is _not_ used if the
>
> res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu);
>
> line above managed to find a comp_dir attribute. In that
> case, res.comp_dir points to some memory owned by the obstack
> that owns DIE/CU, and the storage field is left empty. I see I
> forgot to add some comments. Fixed in this update below.
Here too.
> I shouldn't point res.comp_dir to the storage if ldirname returned an
> empty string though. Fixed now too.
V2 looks good to me.
Philipp
> From 98a4e4d213eb85b264c50bcae7664f5fb3cd29d7 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 22 Mar 2017 15:21:29 +0000
> Subject: [PATCH v2] gdb: Make ldirname return a std::string
>
> Eliminates several uses of cleanups.
>
> (And if applied before
> <https://sourceware.org/ml/gdb-patches/2017-03/msg00394.html>, fixes
> the leak in python.c:do_start_initialization too.)
>
> Tested on x86_64 Fedora 23 with Python 2 and 3.
>
> gdb/ChangeLog
> 2017-03-22 Pedro Alves <palves@redhat.com>
>
> * dwarf2read.c (struct file_and_directory): New.
> (dwarf2_get_dwz_file): Adjust to use std::string.
> (dw2_get_file_names_reader): Adjust to use file_and_directory.
> (find_file_and_directory): Adjust to return a
> file_and_directory object.
> (read_file_scope): Adjust to use file_and_directory. Remove
> make_cleanup/do_cleanups calls.
> (open_and_init_dwp_file): Adjust to use std::string. Remove
> make_cleanup/do_cleanups calls.
> * python/python.c (do_start_initialization): Adjust to
> ldirname returning a std::string.
> * utils.c (ldirname): Now returns a std::string.
> * utils.h (ldirname): Change return type to std::string.
> * xml-syscall.c (xml_init_syscalls_info): Adjust to ldirname
> returning a std::string.
> * xml-tdesc.c (file_read_description_xml): Likewise.
> ---
> gdb/dwarf2read.c | 117
> +++++++++++++++++++++++++++-------------------------
> gdb/python/python.c | 2 +- gdb/utils.c | 10 ++---
> gdb/utils.h | 2 +-
> gdb/xml-syscall.c | 8 +---
> gdb/xml-tdesc.c | 8 +---
> 6 files changed, 71 insertions(+), 76 deletions(-)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index b3ea52b..36b6ead 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1885,9 +1885,27 @@ static void queue_comp_unit (struct
> dwarf2_per_cu_data *per_cu,
>
> static void process_queue (void);
>
> -static void find_file_and_directory (struct die_info *die,
> - struct dwarf2_cu *cu,
> - const char **name, const char
> **comp_dir); +/* The return type of find_file_and_directory. Note,
> the enclosed
> + string pointers are only valid while this object is valid. */
> +
> +struct file_and_directory
> +{
> + /* The filename. This is never NULL. */
> + const char *name;
> +
> + /* The compilation directory. NULL if not known. If we needed to
> + compute a new string, this points to COMP_DIR_STORAGE,
> otherwise,
> + points directly to the DW_AT_comp_dir string attribute owned by
> + the obstack that owns the DIE. */
> + const char *comp_dir;
> +
> + /* If we needed to build a new string for comp_dir, this is what
> + owns the storage. */
> + std::string comp_dir_storage;
> +};
> +
> +static file_and_directory find_file_and_directory (struct die_info
> *die,
> + struct dwarf2_cu
> *cu);
>
> static char *file_full_name (int file, struct line_header *lh,
> const char *comp_dir);
> @@ -2552,18 +2570,15 @@ dwarf2_get_dwz_file (void)
> buildid_len = (size_t) buildid_len_arg;
>
> filename = (const char *) data;
> +
> + std::string abs_storage;
> if (!IS_ABSOLUTE_PATH (filename))
> {
> char *abs = gdb_realpath (objfile_name
> (dwarf2_per_objfile->objfile));
> - char *rel;
>
> make_cleanup (xfree, abs);
> - abs = ldirname (abs);
> - make_cleanup (xfree, abs);
> -
> - rel = concat (abs, SLASH_STRING, filename, (char *) NULL);
> - make_cleanup (xfree, rel);
> - filename = rel;
> + abs_storage = ldirname (abs) + SLASH_STRING + filename;
> + filename = abs_storage.c_str ();
> }
>
> /* First try the file name given in the section. If that doesn't
> @@ -3332,7 +3347,6 @@ dw2_get_file_names_reader (const struct
> die_reader_specs *reader, struct line_header *lh;
> struct attribute *attr;
> int i;
> - const char *name, *comp_dir;
> void **slot;
> struct quick_file_names *qfn;
> unsigned int line_offset;
> @@ -3385,13 +3399,13 @@ dw2_get_file_names_reader (const struct
> die_reader_specs *reader, gdb_assert (slot != NULL);
> *slot = qfn;
>
> - find_file_and_directory (comp_unit_die, cu, &name, &comp_dir);
> + file_and_directory fnd = find_file_and_directory (comp_unit_die,
> cu);
>
> qfn->num_file_names = lh->num_file_names;
> qfn->file_names =
> XOBNEWVEC (&objfile->objfile_obstack, const char *,
> lh->num_file_names); for (i = 0; i < lh->num_file_names; ++i)
> - qfn->file_names[i] = file_full_name (i + 1, lh, comp_dir);
> + qfn->file_names[i] = file_full_name (i + 1, lh, fnd.comp_dir);
> qfn->real_names = NULL;
>
> free_line_header (lh);
> @@ -9122,37 +9136,38 @@ 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);
> + if (!res.comp_dir_storage.empty ())
> + res.comp_dir = res.comp_dir_storage.c_str ();
> }
> - if (*comp_dir != NULL)
> + if (res.comp_dir != NULL)
> {
> /* 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 +9277,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 +9293,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 +9307,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 +9350,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 +10902,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 +10963,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;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] gdb: Make ldirname return a std::string
2017-03-23 10:04 ` Philipp Rudo
@ 2017-03-27 11:12 ` Pedro Alves
0 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2017-03-27 11:12 UTC (permalink / raw)
To: Philipp Rudo; +Cc: gdb-patches
On 03/23/2017 10:04 AM, Philipp Rudo wrote:
> V2 looks good to me.
Thanks. Rebased and pushed, as below.
From d721ba37d8995b9c11a0b8eef0f4d2dc022f85aa Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 27 Mar 2017 11:56:28 +0100
Subject: [PATCH] gdb: Make ldirname return a std::string
Eliminates several uses of cleanups.
Tested on x86_64 Fedora 23 with Python 2 and 3.
gdb/ChangeLog
2017-03-27 Pedro Alves <palves@redhat.com>
* dwarf2read.c (struct file_and_directory): New.
(dwarf2_get_dwz_file): Adjust to use std::string.
(dw2_get_file_names_reader): Adjust to use file_and_directory.
(find_file_and_directory): Adjust to return a file_and_directory
object.
(read_file_scope): Adjust to use file_and_directory. Remove
make_cleanup/do_cleanups calls.
(open_and_init_dwp_file): Adjust to use std::string. Remove
make_cleanup/do_cleanups calls.
* python/python.c (do_start_initialization): Adjust to ldirname
returning a std::string.
* utils.c (ldirname): Now returns a std::string.
* utils.h (ldirname): Change return type to std::string.
* xml-syscall.c (xml_init_syscalls_info): Adjust to ldirname
returning a std::string.
* xml-tdesc.c (file_read_description_xml): Likewise.
---
gdb/ChangeLog | 19 +++++++++
gdb/dwarf2read.c | 117 +++++++++++++++++++++++++++-------------------------
gdb/python/python.c | 4 +-
gdb/utils.c | 10 ++---
gdb/utils.h | 2 +-
gdb/xml-syscall.c | 8 +---
gdb/xml-tdesc.c | 8 +---
7 files changed, 90 insertions(+), 78 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a687fae..82213e8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2017-03-27 Pedro Alves <palves@redhat.com>
+
+ * dwarf2read.c (struct file_and_directory): New.
+ (dwarf2_get_dwz_file): Adjust to use std::string.
+ (dw2_get_file_names_reader): Adjust to use file_and_directory.
+ (find_file_and_directory): Adjust to return a file_and_directory
+ object.
+ (read_file_scope): Adjust to use file_and_directory. Remove
+ make_cleanup/do_cleanups calls.
+ (open_and_init_dwp_file): Adjust to use std::string. Remove
+ make_cleanup/do_cleanups calls.
+ * python/python.c (do_start_initialization): Adjust to ldirname
+ returning a std::string.
+ * utils.c (ldirname): Now returns a std::string.
+ * utils.h (ldirname): Change return type to std::string.
+ * xml-syscall.c (xml_init_syscalls_info): Adjust to ldirname
+ returning a std::string.
+ * xml-tdesc.c (file_read_description_xml): Likewise.
+
2017-03-24 Alan Hayward <alan.hayward@arm.com>
* regcache.c (regcache_debug_print_register): New function.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 519550b..f342950 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1885,9 +1885,27 @@ static void queue_comp_unit (struct dwarf2_per_cu_data *per_cu,
static void process_queue (void);
-static void find_file_and_directory (struct die_info *die,
- struct dwarf2_cu *cu,
- const char **name, const char **comp_dir);
+/* The return type of find_file_and_directory. Note, the enclosed
+ string pointers are only valid while this object is valid. */
+
+struct file_and_directory
+{
+ /* The filename. This is never NULL. */
+ const char *name;
+
+ /* The compilation directory. NULL if not known. If we needed to
+ compute a new string, this points to COMP_DIR_STORAGE, otherwise,
+ points directly to the DW_AT_comp_dir string attribute owned by
+ the obstack that owns the DIE. */
+ const char *comp_dir;
+
+ /* If we needed to build a new string for comp_dir, this is what
+ owns the storage. */
+ std::string comp_dir_storage;
+};
+
+static file_and_directory find_file_and_directory (struct die_info *die,
+ struct dwarf2_cu *cu);
static char *file_full_name (int file, struct line_header *lh,
const char *comp_dir);
@@ -2552,18 +2570,15 @@ dwarf2_get_dwz_file (void)
buildid_len = (size_t) buildid_len_arg;
filename = (const char *) data;
+
+ std::string abs_storage;
if (!IS_ABSOLUTE_PATH (filename))
{
char *abs = gdb_realpath (objfile_name (dwarf2_per_objfile->objfile));
- char *rel;
make_cleanup (xfree, abs);
- abs = ldirname (abs);
- make_cleanup (xfree, abs);
-
- rel = concat (abs, SLASH_STRING, filename, (char *) NULL);
- make_cleanup (xfree, rel);
- filename = rel;
+ abs_storage = ldirname (abs) + SLASH_STRING + filename;
+ filename = abs_storage.c_str ();
}
/* First try the file name given in the section. If that doesn't
@@ -3332,7 +3347,6 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
struct line_header *lh;
struct attribute *attr;
int i;
- const char *name, *comp_dir;
void **slot;
struct quick_file_names *qfn;
unsigned int line_offset;
@@ -3385,13 +3399,13 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
gdb_assert (slot != NULL);
*slot = qfn;
- find_file_and_directory (comp_unit_die, cu, &name, &comp_dir);
+ file_and_directory fnd = find_file_and_directory (comp_unit_die, cu);
qfn->num_file_names = lh->num_file_names;
qfn->file_names =
XOBNEWVEC (&objfile->objfile_obstack, const char *, lh->num_file_names);
for (i = 0; i < lh->num_file_names; ++i)
- qfn->file_names[i] = file_full_name (i + 1, lh, comp_dir);
+ qfn->file_names[i] = file_full_name (i + 1, lh, fnd.comp_dir);
qfn->real_names = NULL;
free_line_header (lh);
@@ -9122,37 +9136,38 @@ 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);
+ if (!res.comp_dir_storage.empty ())
+ res.comp_dir = res.comp_dir_storage.c_str ();
}
- if (*comp_dir != NULL)
+ if (res.comp_dir != NULL)
{
/* 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 +9277,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 +9293,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 +9307,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 +9350,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.
@@ -10893,49 +10903,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;
@@ -10959,7 +10964,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 d21e023..a7aff53 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1550,10 +1550,8 @@ do_start_initialization ()
/foo/bin/python
/foo/lib/pythonX.Y/...
This must be done before calling Py_Initialize. */
- char *libdir = ldirname (python_libdir);
- progname = concat (libdir, SLASH_STRING, "bin",
+ progname = concat (ldirname (python_libdir).c_str (), SLASH_STRING, "bin",
SLASH_STRING, "python", (char *) NULL);
- xfree (libdir);
#ifdef IS_PY3K
oldloc = xstrdup (setlocale (LC_ALL, NULL));
setlocale (LC_ALL, "");
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;
--
2.5.5
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-27 11:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 16:04 [PATCH] gdb: Make ldirname return a std::string Pedro Alves
2017-03-22 18:01 ` Philipp Rudo
2017-03-22 19:26 ` [PATCH v2] " Pedro Alves
2017-03-23 10:04 ` Philipp Rudo
2017-03-27 11:12 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).