This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] dangling cleanup in find_frame_funname


> 2013-05-16  Keith Seitz  <keiths@redhat.com>
> 
> 	* ada-lang.c (is_known_support_routine): Add explicit free of
> 	'func_name' from find_frame_funname.
> 	(ada_unhandled_exception_name_addr_from_raise): Likewise.
> 	* python/py-frame.c (frapy_name): Likewise for 'name'.
> 	* stack.c (find_frame_funname): Add comment explaining that
> 	funcp must be freed by the caller.
> 	Return copy of symbol names instead of pointers.
> 	(print_frame): Add a cleanup for 'funname' from
> 	find_frame_funname.
> 	* stack.h (find_frame_funname): Remove "const" from
> 	'funname' parameter.

My two cents...

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index fa6db0f..12e5388 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -11105,7 +11105,7 @@ static int
>  is_known_support_routine (struct frame_info *frame)
>  {
>    struct symtab_and_line sal;
> -  const char *func_name;
> +  char *func_name;
>    enum language func_lang;
>    int i;
>    const char *fullname;
> @@ -11152,9 +11152,13 @@ is_known_support_routine (struct frame_info *frame)
>      {
>        re_comp (known_auxiliary_function_name_patterns[i]);
>        if (re_exec (func_name))
> -        return 1;
> +	{
> +	  xfree (func_name);
> +	  return 1;
> +	}
>      }
>  
> +  xfree (func_name);
>    return 0;
>  }

As long as we only have one variable to xfree, not using a cleanup
is ok...

> @@ -11210,13 +11214,17 @@ ada_unhandled_exception_name_addr_from_raise (void)
>  
>    while (fi != NULL)
>      {
> -      const char *func_name;
> +      char *func_name;
>        enum language func_lang;
>  
>        find_frame_funname (fi, &func_name, &func_lang, NULL);
>        if (func_name != NULL
>            && strcmp (func_name, data->exception_info->catch_exception_sym) == 0)
> -        break; /* We found the frame we were looking for...  */
> +	{
> +	  xfree (func_name);
> +	  break; /* We found the frame we were looking for...  */
> +	}
> +      xfree (func_name);
>        fi = get_prev_frame (fi);
>      }

In this case, I am leaning towards using a cleanup, something like this:

  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
  while (fi != NULL)
    {
      char *func_name;
      enum language func_lang;

      find_frame_funname (fi, &func_name, &func_lang, NULL);
      if (func_name != NULL)
        make_cleanup (xfree, func_name);
      if (func_name != NULL
          && strcmp (func_name, data->exception_info->catch_exception_sym) == 0)
        break; /* We found the frame we were looking for...  */
      fi = get_prev_frame (fi);
    }
  do_cleanups (old_chain);

I find the code a little simpler to maintain that way. WDYT?

The Ada section looks OK to me either way.

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]