Re: [PATCH 27/40] Make cp_remove_params return a unique_ptr

Pedro Alves
2017-10-09T15:13:43Z

On 08/08/2017 09:35 PM, Keith Seitz wrote:
> On 06/02/2017 05:22 AM, Pedro Alves wrote:
>> gdb/ChangeLog:
>> yyyy-mm-dd Pedro Alves <palves@redhat.com>
>>
>> (overload_list_add_symbol): Adjust to use gdb::unique_xmalloc_ptr.
>> * cp-support.h (cp_remove_params): Now returns a
>> gdb::unique_xmalloc_ptr.
>> * dwarf2read.c (find_slot_in_mapped_hash): Now returns bool.
>> Adjust to use gdb::unique_xmalloc_ptr.
>> (dw2_expand_symtabs_matching_symbol): Adjust to use
>> gdb::unique_xmalloc_ptr.
>> * psymtab.c (psymtab_search_name): Now returns a
>> gdb::unique_xmalloc_ptr.
>> (lookup_partial_symbol): Adjust to use gdb::unique_xmalloc_ptr.
>> * stack.c (find_frame_funname): Adjust to use
>> gdb::unique_xmalloc_ptr.
>
> LGTM
>

Thanks Keith.

I neglected applying this promptly, and meanwhile the cleanups
have been replaced in master via other patches.

The change to return unique_ptr is still useful on its own IMO,
so I've now pushed the remainder of the patch to master, as below.

From 109483d9eec3f0d0c3eaafd5d829435059167c52 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 9 Oct 2017 15:57:36 +0100
Subject: [PATCH] Make cp_remove_params return a gdb::unique_xmalloc_ptr

Use the type system instead of callers needing to know how the
returned string's memory is supposed to be managed.

gdb/ChangeLog:
2017-10-09 Pedro Alves <palves@redhat.com>

* cp-support.c (cp_remove_params): Return a gdb::unique_xmalloc_ptr.
Use bool.
(overload_list_add_symbol): Adjust to use gdb::unique_xmalloc_ptr.
* cp-support.h (cp_remove_params): Now returns a
gdb::unique_xmalloc_ptr.
* dwarf2read.c (find_slot_in_mapped_hash): Now returns bool.
Adjust to cp_remove_params returning a gdb::unique_xmalloc_ptr.
* psymtab.c (psymtab_search_name): Adjust to cp_remove_params
returning a gdb::unique_xmalloc_ptr.
(lookup_partial_symbol): Adjust to use gdb::unique_xmalloc_ptr.
* stack.c (find_frame_funname): Adjust to cp_remove_params
returning a gdb::unique_xmalloc_ptr.
---
gdb/ChangeLog | 15 +++++++++++++++
gdb/cp-support.c | 19 +++++++------------
gdb/cp-support.h | 3 ++-
gdb/dwarf2read.c | 11 ++++++-----
gdb/psymtab.c | 8 +++-----
gdb/stack.c | 7 ++-----
6 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a21f8a0..9bf5f6e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2017-10-09 Pedro Alves <palves@redhat.com>
+
+ * cp-support.c (cp_remove_params): Return a gdb::unique_xmalloc_ptr.
+ Use bool.
+ (overload_list_add_symbol): Adjust to use gdb::unique_xmalloc_ptr.
+ * cp-support.h (cp_remove_params): Now returns a
+ gdb::unique_xmalloc_ptr.
+ * dwarf2read.c (find_slot_in_mapped_hash): Now returns bool.
+ Adjust to cp_remove_params returning a gdb::unique_xmalloc_ptr.
+ * psymtab.c (psymtab_search_name): Adjust to cp_remove_params
+ returning a gdb::unique_xmalloc_ptr.
+ (lookup_partial_symbol): Adjust to use gdb::unique_xmalloc_ptr.
+ * stack.c (find_frame_funname): Adjust to cp_remove_params
+ returning a gdb::unique_xmalloc_ptr.
+
2017-10-08 Tom Tromey <tom@tromey.com>

* dwarf2read.c (dwarf2_get_dwz_file): Use
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index d88bdaa..7bcb155 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -838,10 +838,10 @@ cp_func_name (const char *full_name)
(optionally) a return type. Return the name of the function without
parameters or return type, or NULL if we can not parse the name. */

-char *
+gdb::unique_xmalloc_ptr<char>
cp_remove_params (const char *demangled_name)
{
- int done = 0;
+ bool done = false;
struct demangle_component *ret_comp;
std::unique_ptr<demangle_parse_info> info;
gdb::unique_xmalloc_ptr<char> ret;
@@ -868,7 +868,7 @@ cp_remove_params (const char *demangled_name)
ret_comp = d_left (ret_comp);
break;
default:
- done = 1;
+ done = true;
break;
}

@@ -876,7 +876,7 @@ cp_remove_params (const char *demangled_name)
if (ret_comp->type == DEMANGLE_COMPONENT_TYPED_NAME)
ret = cp_comp_to_string (d_left (ret_comp), 10);

- return ret.release ();
+ return ret;
}

/* Here are some random pieces of trivia to keep in mind while trying
@@ -1103,7 +1103,7 @@ overload_list_add_symbol (struct symbol *sym,
{
int newsize;
int i;
- char *sym_name;
+ gdb::unique_xmalloc_ptr<char> sym_name;

/* If there is no type information, we can't do anything, so
skip. */
@@ -1122,13 +1122,8 @@ overload_list_add_symbol (struct symbol *sym,
return;

/* skip symbols that cannot match */
- if (strcmp (sym_name, oload_name) != 0)
- {
- xfree (sym_name);
- return;
- }
-
- xfree (sym_name);
+ if (strcmp (sym_name.get (), oload_name) != 0)
+ return;

/* We have a match for an overload instance, so add SYM to the
current list of overload instances */
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 9210165..28353a2 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -95,7 +95,8 @@ extern unsigned int cp_entire_prefix_len (const char *name);

extern char *cp_func_name (const char *full_name);

-extern char *cp_remove_params (const char *demangled_name);
+extern gdb::unique_xmalloc_ptr<char> cp_remove_params
+ (const char *demanged_name);

extern struct symbol **make_symbol_overload_list (const char *,
const char *);
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3b90359..ca5b3a8 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3194,9 +3194,10 @@ mapped_index_string_hash (int index_version, const void *p)

/* Find a slot in the mapped index INDEX for the object named NAME.
If NAME is found, set *VEC_OUT to point to the CU vector in the
- constant pool and return 1. If NAME cannot be found, return 0. */
+ constant pool and return true. If NAME cannot be found, return
+ false. */

-static int
+static bool
find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
offset_type **vec_out)
{
@@ -3214,7 +3215,7 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,

if (strchr (name, '(') != NULL)
{
- without_params.reset (cp_remove_params (name));
+ without_params = cp_remove_params (name);

if (without_params != NULL)
name = without_params.get ();
@@ -3239,14 +3240,14 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
offset_type i = 2 * slot;
const char *str;
if (index->symbol_table[i] == 0 && index->symbol_table[i + 1] == 0)
- return 0;
+ return false;

str = index->constant_pool + MAYBE_SWAP (index->symbol_table[i]);
if (!cmp (name, str))
{
*vec_out = (offset_type *) (index->constant_pool
+ MAYBE_SWAP (index->symbol_table[i + 1]));
- return 1;
+ return true;
}

slot = (slot + step) & (index->symbol_table_slots - 1);
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 4527d69..f55c98c 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -623,9 +623,7 @@ match_partial_symbol (struct objfile *objfile,
not contain any method/function instance information (since this would
force reading type information while reading psymtabs). Therefore,
if NAME contains overload information, it must be stripped before searching
- psymtabs.
-
- The caller is responsible for freeing the return result. */
+ psymtabs. */

static gdb::unique_xmalloc_ptr<char>
psymtab_search_name (const char *name)
@@ -636,10 +634,10 @@ psymtab_search_name (const char *name)
{
if (strchr (name, '('))
{
- char *ret = cp_remove_params (name);
+ gdb::unique_xmalloc_ptr<char> ret = cp_remove_params (name);

if (ret)
- return gdb::unique_xmalloc_ptr<char> (ret);
+ return ret;
}
}
break;
diff --git a/gdb/stack.c b/gdb/stack.c
index 53dc829..4e40e32 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1101,10 +1101,7 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
stored in the symbol table, but we stored a version
with DMGL_PARAMS turned on, and here we don't want to
display parameters. So remove the parameters. */
- char *func_only = cp_remove_params (print_name);
-
- if (func_only)
- funname.reset (func_only);
+ funname = cp_remove_params (print_name);
}

/* If we didn't hit the C++ case above, set *funname
@@ -1434,7 +1431,7 @@ info_frame_command (char *addr_exp, int from_tty)
stored in the symbol table, but we stored a version
with DMGL_PARAMS turned on, and here we don't want to
display parameters. So remove the parameters. */
- func_only.reset (cp_remove_params (funname));
+ func_only = cp_remove_params (funname);

if (func_only)
funname = func_only.get ();
--
2.5.5