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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Fri, 29 Jul 2011 01:17:46 +0200, Keith Seitz wrote:
> diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
> index 8736777..f745a31 100644
> --- a/gdb/cp-name-parser.y
> +++ b/gdb/cp-name-parser.y
[...]
> +/* 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);
> +}

$ make test-cp-name-parser
cp-name-parser.y:2018: undefined reference to `make_cleanup'


[...]
> -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 = ((struct demangle_parse_info *)
> +	    malloc (sizeof (struct demangle_parse_info)));

This cast seems redundant to me.


[...]
> @@ -153,23 +154,29 @@ 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 = ((struct demangle_parse_info *)
> +		  xmalloc (sizeof (struct demangle_parse_info)));

Redundant cast.


[...]
> --- 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;
> +};

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.



Thanks,
Jan