Re: [RFA] c++/12266 (again) [cp_demangled_name_parse_free-4.patch]

[Keith Seitz <keiths@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

On 08/02/2011 01:28 PM, Jan Kratochvil wrote:
> $ make test-cp-name-parser
> cp-name-parser.y:2018: undefined reference to `make_cleanup'

Fixed.

>> +  result = ((struct demangle_parse_info *)
>> +	    malloc (sizeof (struct demangle_parse_info)));
>
> This cast seems redundant to me.

Removed. GDB is a mish-mash of usage discrepancies like this. [Mind you, 
a C++ compiler will complain about this.]

>> +	  info = ((struct demangle_parse_info *)
>> +		  xmalloc (sizeof (struct demangle_parse_info)));
>
> Redundant cast.

Removed.

> I guess you have been considering it but anyway:
>
> Its ->tree items can reference for s_name the original string being demangled,
> as you even state in some comments.  Due to it it also requires later explicit
> handling of `free_list'.  Could this struct already have the original string
> duplicated and this struct would track its freeing?
> cp_merge_demangle_parse_infos would then track all the source strings in the
> merged destination demangle_parse_info.

That is actually something that didn't occur to me for some reason, but 
I've made the necessary changes to use an obstack. [This change can be 
seen in the next patch, since it is not strictly necessary for this 
cleanup/refactoring.]

New patch attached.

Keith

ChangeLog
2011-08-09  Keith Seitz  <keiths@redhat.com>

	* cp-name-parser.y (struct demangle_info): Remove unused
	member PREV.
	(d_grab): Likewise.
	(allocate_info): Change return type to struct demangle_info *.
	Always allocate a new demangle_info.
	Remove unused PREV pointer.
	(cp_new_demangle_parse_info): New function.
	(cp_demangled_name_parse_free): New function.
  	(do_demangled_name_parse_free_cleanup): New function.
	(make_cleanup_cp_demangled_name_parse_free): New function.
	(cp_demangled_name_to_comp): Change return type to
	struct demangle_parse_info *.
	Allocate a new storage for each call.
	(main): Update usage for cp_demangled_name_to_comp
	API change.
	* cp-support.h (struct demangle_parse_info): New structure.
	(cp_demangled_name_to_comp): Update API change for
	return type.
	(cp_new_demangle_parse_info): Declare.
	(make_cleanup_cp_demangled_name_parse_free): New declaration.
	(cp_demangled_name_parse_free): Declare.
	* cp-support.c (cp_canonicalize_string): Update API
	change for cp_demangled_name_to_comp.
	(mangled_name_to_comp): Likewise.
	Return struct demangle_parse_info, too.
	(cp_class_name_from_physname): Update mangled_name_to_comp
	API change.
	(method_name_from_physname): Likewise.
	(cp_func_name): Update API change for cp_demangled_name_to_comp.
	(cp_remove_params): Likewise.
	* python/py-type.c (typy_legacy_template_argument): Likewise.


[-- Attachment #2: cp_demangled_name_parse_free-5.patch --]
[-- Type: text/plain, Size: 13678 bytes --]

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 8736777..6d23c9d 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -60,7 +60,7 @@ static const char *lexptr, *prev_lexptr, *error_lexptr, *global_errmsg;
 
 struct demangle_info {
   int used;
-  struct demangle_info *prev, *next;
+  struct demangle_info *next;
   struct demangle_component comps[ALLOC_CHUNK];
 };
 
@@ -76,7 +76,6 @@ d_grab (void)
       if (demangle_info->next == NULL)
 	{
 	  more = malloc (sizeof (struct demangle_info));
-	  more->prev = demangle_info;
 	  more->next = NULL;
 	  demangle_info->next = more;
 	}
@@ -1935,20 +1934,14 @@ yyerror (char *msg)
    generally allocate too many components, but the extra memory usage
    doesn't hurt because the trees are temporary and the storage is
    reused.  More may be allocated later, by d_grab.  */
-static void
+static struct demangle_info *
 allocate_info (void)
 {
-  if (demangle_info == NULL)
-    {
-      demangle_info = malloc (sizeof (struct demangle_info));
-      demangle_info->prev = NULL;
-      demangle_info->next = NULL;
-    }
-  else
-    while (demangle_info->prev)
-      demangle_info = demangle_info->prev;
+  struct demangle_info *info = malloc (sizeof (struct demangle_info));
 
-  demangle_info->used = 0;
+  info->next = NULL;
+  info->used = 0;
+  return info;
 }
 
 /* Convert RESULT to a string.  The return value is allocated
@@ -1966,23 +1959,60 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len)
 			       &err);
 }
 
+/* A convenience function to allocate and initialize a new struct
+   demangled_parse_info.  */
+
+struct demangle_parse_info *
+cp_new_demangle_parse_info (void)
+{
+  struct demangle_parse_info *info;
+
+  info = malloc (sizeof (struct demangle_parse_info));
+  info->info = NULL;
+  info->tree = NULL;
+  return info;
+}
+
+/* Free any memory associated with the given PARSE_INFO.  */
+
+void
+cp_demangled_name_parse_free (struct demangle_parse_info *parse_info)
+{
+  struct demangle_info *info = parse_info->info;
+
+  /* Free any allocated chunks of memory for the parse.  */
+  while (info != NULL)
+    {
+      struct demangle_info *next = info->next;
+
+      free (info);
+      info = next;
+    }
+
+  /* Free the parser info.  */
+  free (parse_info);
+}
+
 /* Convert a demangled name to a demangle_component tree.  On success,
-   the root of the new tree is returned; it is valid until the next
-   call to this function and should not be freed.  On error, NULL is
+   a structure containing the root of the new tree is returned; it must
+   be freed by calling cp_demangled_name_parse_free. On error, NULL is
    returned, and an error message will be set in *ERRMSG (which does
    not need to be freed).  */
 
-struct demangle_component *
+struct demangle_parse_info *
 cp_demangled_name_to_comp (const char *demangled_name, const char **errmsg)
 {
   static char errbuf[60];
-  struct demangle_component *result;
+  struct demangle_parse_info *result;
 
   prev_lexptr = lexptr = demangled_name;
   error_lexptr = NULL;
   global_errmsg = NULL;
 
-  allocate_info ();
+  demangle_info = allocate_info ();
+
+  result = cp_new_demangle_parse_info ();
+  result->info = demangle_info;
 
   if (yyparse ())
     {
@@ -1993,10 +2023,11 @@ cp_demangled_name_to_comp (const char *demangled_name, const char **errmsg)
 	  strcat (errbuf, "'");
 	  *errmsg = errbuf;
 	}
+      cp_demangled_name_parse_free (result);
       return NULL;
     }
 
-  result = global_result;
+  result->tree = global_result;
   global_result = NULL;
 
   return result;
@@ -2052,7 +2083,7 @@ main (int argc, char **argv)
   char buf[65536];
   int arg;
   const char *errmsg;
-  struct demangle_component *result;
+  struct demangle_parse_info *result;
 
   arg = 1;
   if (argv[arg] && strcmp (argv[arg], "--debug") == 0)
@@ -2086,7 +2117,8 @@ main (int argc, char **argv)
 	    continue;
 	  }
 
-	cp_print (result);
+	cp_print (result->tree);
+	cp_demangled_name_parse_free (result);
 
 	free (str2);
 	if (c)
@@ -2105,7 +2137,8 @@ main (int argc, char **argv)
 	  fputc ('\n', stderr);
 	  return 0;
 	}
-      cp_print (result);
+      cp_print (result->tree);
+      cp_demangled_name_parse_free (result);
       putchar ('\n');
     }
   return 0;
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index a479067..8cda2b4 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -86,6 +86,25 @@ static const char *operator_tokens[] =
     /* new[] and delete[] require special whitespace handling */
   };
 
+
+/* A cleanup wrapper for cp_demangled_name_parse_free.  */
+
+static void
+do_demangled_name_parse_free_cleanup (void *data)
+{
+  struct demangle_parse_info *info = (struct demangle_parse_info *) data;
+
+  cp_demangled_name_parse_free (info);
+}
+
+/* Create a cleanup for C++ name parsing.  */
+
+struct cleanup *
+make_cleanup_cp_demangled_name_parse_free (struct demangle_parse_info *info)
+{
+  return make_cleanup (do_demangled_name_parse_free_cleanup, info);
+}
+
 /* Return 1 if STRING is clearly already in canonical form.  This
    function is conservative; things which it does not recognize are
    assumed to be non-canonical, and the parser will sort them out
@@ -124,19 +143,20 @@ cp_already_canonical (const char *string)
 char *
 cp_canonicalize_string (const char *string)
 {
-  struct demangle_component *ret_comp;
+  struct demangle_parse_info *info;
   unsigned int estimated_len;
   char *ret;
 
   if (cp_already_canonical (string))
     return NULL;
 
-  ret_comp = cp_demangled_name_to_comp (string, NULL);
-  if (ret_comp == NULL)
+  info = cp_demangled_name_to_comp (string, NULL);
+  if (info == NULL)
     return NULL;
 
   estimated_len = strlen (string) * 2;
-  ret = cp_comp_to_string (ret_comp, estimated_len);
+  ret = cp_comp_to_string (info->tree, estimated_len);
+  cp_demangled_name_parse_free (info);
 
   if (strcmp (string, ret) == 0)
     {
@@ -153,23 +173,27 @@ cp_canonicalize_string (const char *string)
    freed when finished with the tree, or NULL if none was needed.
    OPTIONS will be passed to the demangler.  */
 
-static struct demangle_component *
+static struct demangle_parse_info *
 mangled_name_to_comp (const char *mangled_name, int options,
 		      void **memory, char **demangled_p)
 {
-  struct demangle_component *ret;
   char *demangled_name;
+  struct demangle_parse_info *info;
 
   /* If it looks like a v3 mangled name, then try to go directly
      to trees.  */
   if (mangled_name[0] == '_' && mangled_name[1] == 'Z')
     {
+      struct demangle_component *ret;
+
       ret = cplus_demangle_v3_components (mangled_name,
 					  options, memory);
       if (ret)
 	{
+	  info = cp_new_demangle_parse_info ();
+	  info->tree = ret;
 	  *demangled_p = NULL;
-	  return ret;
+	  return info;
 	}
     }
 
@@ -181,16 +205,16 @@ mangled_name_to_comp (const char *mangled_name, int options,
   
   /* If we could demangle the name, parse it to build the component
      tree.  */
-  ret = cp_demangled_name_to_comp (demangled_name, NULL);
+  info = cp_demangled_name_to_comp (demangled_name, NULL);
 
-  if (ret == NULL)
+  if (info == NULL)
     {
       xfree (demangled_name);
       return NULL;
     }
 
   *demangled_p = demangled_name;
-  return ret;
+  return info;
 }
 
 /* Return the name of the class containing method PHYSNAME.  */
@@ -201,14 +225,16 @@ cp_class_name_from_physname (const char *physname)
   void *storage = NULL;
   char *demangled_name = NULL, *ret;
   struct demangle_component *ret_comp, *prev_comp, *cur_comp;
+  struct demangle_parse_info *info;
   int done;
 
-  ret_comp = mangled_name_to_comp (physname, DMGL_ANSI,
-				   &storage, &demangled_name);
-  if (ret_comp == NULL)
+  info = mangled_name_to_comp (physname, DMGL_ANSI,
+			       &storage, &demangled_name);
+  if (info == NULL)
     return NULL;
 
   done = 0;
+  ret_comp = info->tree;
 
   /* First strip off any qualifiers, if we have a function or
      method.  */
@@ -277,8 +303,8 @@ cp_class_name_from_physname (const char *physname)
     }
 
   xfree (storage);
-  if (demangled_name)
-    xfree (demangled_name);
+  xfree (demangled_name);
+  cp_demangled_name_parse_free (info);
   return ret;
 }
 
@@ -348,13 +374,14 @@ method_name_from_physname (const char *physname)
   void *storage = NULL;
   char *demangled_name = NULL, *ret;
   struct demangle_component *ret_comp;
+  struct demangle_parse_info *info;
 
-  ret_comp = mangled_name_to_comp (physname, DMGL_ANSI,
-				   &storage, &demangled_name);
-  if (ret_comp == NULL)
+  info = mangled_name_to_comp (physname, DMGL_ANSI,
+			       &storage, &demangled_name);
+  if (info == NULL)
     return NULL;
 
-  ret_comp = unqualified_name_from_comp (ret_comp);
+  ret_comp = unqualified_name_from_comp (info->tree);
 
   ret = NULL;
   if (ret_comp != NULL)
@@ -363,8 +390,8 @@ method_name_from_physname (const char *physname)
     ret = cp_comp_to_string (ret_comp, 10);
 
   xfree (storage);
-  if (demangled_name)
-    xfree (demangled_name);
+  xfree (demangled_name);
+  cp_demangled_name_parse_free (info);
   return ret;
 }
 
@@ -379,17 +406,19 @@ cp_func_name (const char *full_name)
 {
   char *ret;
   struct demangle_component *ret_comp;
+  struct demangle_parse_info *info;
 
-  ret_comp = cp_demangled_name_to_comp (full_name, NULL);
-  if (!ret_comp)
+  info = cp_demangled_name_to_comp (full_name, NULL);
+  if (!info)
     return NULL;
 
-  ret_comp = unqualified_name_from_comp (ret_comp);
+  ret_comp = unqualified_name_from_comp (info->tree);
 
   ret = NULL;
   if (ret_comp != NULL)
     ret = cp_comp_to_string (ret_comp, 10);
 
+  cp_demangled_name_parse_free (info);
   return ret;
 }
 
@@ -402,16 +431,18 @@ cp_remove_params (const char *demangled_name)
 {
   int done = 0;
   struct demangle_component *ret_comp;
+  struct demangle_parse_info *info;
   char *ret = NULL;
 
   if (demangled_name == NULL)
     return NULL;
 
-  ret_comp = cp_demangled_name_to_comp (demangled_name, NULL);
-  if (ret_comp == NULL)
+  info = cp_demangled_name_to_comp (demangled_name, NULL);
+  if (info == NULL)
     return NULL;
 
   /* First strip off any qualifiers, if we have a function or method.  */
+  ret_comp = info->tree;
   while (!done)
     switch (ret_comp->type)
       {
@@ -433,6 +464,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);
 
+  cp_demangled_name_parse_free (info);
   return ret;
 }
 
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index d23f19e..f333ffa 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -46,6 +46,17 @@ struct demangle_component;
 
 #define CP_ANONYMOUS_NAMESPACE_LEN 21
 
+/* The result of parsing a name.  */
+
+struct demangle_parse_info
+{
+  /* The memory used during the parse.  */
+  struct demangle_info *info;
+
+  /* The result of the parse.  */
+  struct demangle_component *tree;
+};
+
 /* This struct is designed to store data from using directives.  It
    says that names from namespace IMPORT_SRC should be visible within
    namespace IMPORT_DEST.  These form a linked list; NEXT is the next
@@ -214,12 +225,18 @@ struct type *cp_lookup_transparent_type (const char *name);
 
 /* Functions from cp-name-parser.y.  */
 
-extern struct demangle_component *cp_demangled_name_to_comp
-  (const char *demangled_name, const char **errmsg);
+extern struct demangle_parse_info *cp_demangled_name_to_comp
+     (const char *demangled_name, const char **errmsg);
 
 extern char *cp_comp_to_string (struct demangle_component *result,
 				int estimated_len);
 
+extern void cp_demangled_name_parse_free (struct demangle_parse_info *);
+extern struct cleanup *make_cleanup_cp_demangled_name_parse_free
+     (struct demangle_parse_info *);
+
+extern struct demangle_parse_info *cp_new_demangle_parse_info (void);
+
 /* The list of "maint cplus" commands.  */
 
 extern struct cmd_list_element *maint_cplus_cmd_list;
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8ab18cf..335342e 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -577,8 +577,10 @@ typy_legacy_template_argument (struct type *type, struct block *block,
 {
   int i;
   struct demangle_component *demangled;
+  struct demangle_parse_info *info;
   const char *err;
   struct type *argtype;
+  struct cleanup *cleanup;
 
   if (TYPE_NAME (type) == NULL)
     {
@@ -587,12 +589,14 @@ typy_legacy_template_argument (struct type *type, struct block *block,
     }
 
   /* Note -- this is not thread-safe.  */
-  demangled = cp_demangled_name_to_comp (TYPE_NAME (type), &err);
-  if (! demangled)
+  info = cp_demangled_name_to_comp (TYPE_NAME (type), &err);
+  if (! info)
     {
       PyErr_SetString (PyExc_RuntimeError, err);
       return NULL;
     }
+  demangled = info->tree;
+  cleanup = make_cleanup_cp_demangled_name_parse_free (info);
 
   /* Strip off component names.  */
   while (demangled->type == DEMANGLE_COMPONENT_QUAL_NAME
@@ -601,6 +605,7 @@ typy_legacy_template_argument (struct type *type, struct block *block,
 
   if (demangled->type != DEMANGLE_COMPONENT_TEMPLATE)
     {
+      do_cleanups (cleanup);
       PyErr_SetString (PyExc_RuntimeError, _("Type is not a template."));
       return NULL;
     }
@@ -613,12 +618,14 @@ typy_legacy_template_argument (struct type *type, struct block *block,
 
   if (! demangled)
     {
+      do_cleanups (cleanup);
       PyErr_Format (PyExc_RuntimeError, _("No argument %d in template."),
 		    argno);
       return NULL;
     }
 
   argtype = typy_lookup_type (demangled->u.s_binary.left, block);
+  do_cleanups (cleanup);
   if (! argtype)
     return NULL;