Re: [RFA] PR python/18565 - make Frame.function work for inline frames

[Pedro Alves <palves@redhat.com>]

On 07/25/2016 04:01 PM, Tom Tromey wrote:

> 	  *funname = xstrdup (SYMBOL_PRINT_NAME (func));
> [...]
> 	  if (*funlang == language_cplus)
> 	    {
> 	      /* It seems appropriate to use SYMBOL_PRINT_NAME() here,
> 		 to display the demangled name that we already have
> 		 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 (*funname);
> 
> I'm not 100% sure that cp_remove_params cannot throw.  
> However, it's
> simple to deal with this by adding a cleanup in find_frame_funname.  I'm
> happy to do this if desired.
> 
> Another approach might be to have a free_current_contents cleanup at the
> start of find_frame_funname and discard it at the exit.  This would
> maybe make it a bit safer in the face of future changes.

Yet another approach would be to push the xstrdup call to after the
cp_remove_params call, and remove the xfree call, something like:

	  if (*funlang == language_cplus)
	    {
	      char *func_only = cp_remove_params (SYMBOL_PRINT_NAME (func));

	      if (func_only)
		*funname = func_only;
	      else
		*funname = xstrdup (SYMBOL_PRINT_NAME (func));
	    }
	  else
	    *funname = xstrdup (SYMBOL_PRINT_NAME (func));
	}


In any case, IMO this would be the subject of a separate patch.

> Alternatively, if we need a try/catch in the caller to possibly free the
> function name, then several other callers are incorrect (ada-lang.c and
> stack.c).

Yeah.  I think that if a function has such a requirement, then it needs
to be clearly documented as that being part of its API contract.
Otherwise, it's too easy for the called function to change in a way that
makes the caller try to free a dangling output pointer.

Thanks,
Pedro Alves