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

[Pedro Alves <palves@redhat.com>]

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