Re: [PATCH 1/2] gdb: get_frame_language now takes a frame parameter.

[Pedro Alves <palves at redhat dot com>]

On 08/04/2015 04:40 PM, Andrew Burgess wrote:
> As part of a drive to remove deprecated_safe_get_selected_frame, make
> the get_frame_language function take a frame parameter.  Given the name
> of the function this actually seems to make a lot of sense.
> 
> The task of fetching a suitable frame is then passed to the calling
> functions.  For get_frame_language there are not many callers, these are
> updated to get the selected frame in a suitable way.

Thanks.

> 
> gdb/ChangeLog:
> 
> 	* language.c (show_language_command): Find selected frame before
> 	asking for the language of that frame.
> 	(set_language_command): Likewise.
> 	* language.h (get_frame_language): Add frame parameter.
> 	* stack.c (get_frame_language): Add frame parameter, assert
> 	parameter is not NULL, update comment and reindent.
> 	* top.c (check_frame_language_change): Pass the selected frame
> 	into get_frame_language.
> ---
>  gdb/ChangeLog  | 11 +++++++++++
>  gdb/language.c | 33 +++++++++++++++++++++++--------
>  gdb/language.h |  2 +-
>  gdb/stack.c    | 61 ++++++++++++++++++++++++++++------------------------------
>  gdb/top.c      |  7 ++++---
>  5 files changed, 70 insertions(+), 44 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index af41072..223d2e5 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,14 @@
> +2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* language.c (show_language_command): Find selected frame before
> +	asking for the language of that frame.
> +	(set_language_command): Likewise.
> +	* language.h (get_frame_language): Add frame parameter.
> +	* stack.c (get_frame_language): Add frame parameter, assert
> +	parameter is not NULL, update comment and reindent.
> +	* top.c (check_frame_language_change): Pass the selected frame
> +	into get_frame_language.
> +
>  2015-08-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
>  
>  	* infcmd.c (signal_command): Call do_cleanups for args_chain.
> diff --git a/gdb/language.c b/gdb/language.c
> index a8b432e..989a8da 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -118,7 +118,8 @@ static void
>  show_language_command (struct ui_file *file, int from_tty,
>  		       struct cmd_list_element *c, const char *value)
>  {
> -  enum language flang;		/* The language of the current frame.  */
> +  struct frame_info *frame;
> +  enum language flang;		/* The language of the frame.  */
>  
>    if (language_mode == language_mode_auto)
>      fprintf_filtered (gdb_stdout,
> @@ -130,11 +131,15 @@ show_language_command (struct ui_file *file, int from_tty,
>  		      _("The current source language is \"%s\".\n"),
>  		      current_language->la_name);
>  
> -  flang = get_frame_language ();
> -  if (flang != language_unknown &&
> -      language_mode == language_mode_manual &&
> -      current_language->la_language != flang)
> -    printf_filtered ("%s\n", lang_frame_mismatch_warn);
> +  if (target_has_execution)

target_has_execution can't be right.  What about core files?

> +    {
> +      frame = get_selected_frame (NULL);
> +      flang = get_frame_language (frame);
> +      if (flang != language_unknown &&
> +	  language_mode == language_mode_manual &&
> +	  current_language->la_language != flang)

Please put the &&'s at the beginning of the next line while at it.

> +	printf_filtered ("%s\n", lang_frame_mismatch_warn);
> +    }
>  }
>  


> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -329,10 +329,11 @@ void
>  check_frame_language_change (void)
>  {
>    static int warned = 0;
> +  struct frame_info *frame;
>  
>    /* First make sure that a new frame has been selected, in case the
>       command or the hooks changed the program state.  */
> -  deprecated_safe_get_selected_frame ();
> +  frame = deprecated_safe_get_selected_frame ();
>    if (current_language != expected_language)
>      {
>        if (language_mode == language_mode_auto && info_verbose)
> @@ -348,11 +349,11 @@ check_frame_language_change (void)
>    /* FIXME: This should be cacheing the frame and only running when
>       the frame changes.  */
>  
> -  if (has_stack_frames ())
> +  if (has_stack_frames () && frame != NULL)

If has_stack_frames() returns true, how can FRAME be NULL?

>      {
>        enum language flang;
>  
> -      flang = get_frame_language ();
> +      flang = get_frame_language (frame);
>        if (!warned
>  	  && flang != language_unknown
>  	  && flang != current_language->la_language)
> 

Thanks,
Pedro Alves