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


> I'm all for making things easier to maintain! :-)

Cool :)!

> 2013-05-20  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): Add cleanups
> 	for func_name from find_frame_funname.
> 	* python/py-frame.c (frapy_name): Add explicit free of
> 	'name' from find_frame_funname.
> 	* 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.

Looks good to me too. Just one tiny request, if I may:

> @@ -11198,6 +11202,7 @@ ada_unhandled_exception_name_addr_from_raise (void)
>    int frame_level;
>    struct frame_info *fi;
>    struct ada_inferior_data *data = get_ada_inferior_data (current_inferior ());
> +  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
>  
>    /* To determine the name of this exception, we need to select
>       the frame corresponding to RAISE_SYM_NAME.  This frame is
> @@ -11210,15 +11215,21 @@ 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...  */
> -      fi = get_prev_frame (fi);
> +      if (func_name != NULL)
> +	{
> +	  make_cleanup (xfree, func_name);
> +
> +          if (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);

Would you mind moving the call to make_cleanup that sets old_chain
just before the "while". To me, that's useful, because the make_cleanup/
do_cleanup calls bracket the while loop, showing that this is the
context where the cleanup is used.

And while touching this area of the code, the "if (strcmp" line is
indented with spaces-only, while it should be tabs-and-spaces
(a PIA, if you ask me).

Thank you!
-- 
Joel


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