Fix build breakage from recent patches

  The recent patches related to making introduced buld failures when
trying to build GDB with guile support.  This posed a non-trivial
problem to fix since guile handles exceptions using longjmp, so we
couldn't just blindly throw frame_info_ptr everywhere.

The fix makes minimal use of the smart pointer wrapper only using it in
function calls or in scopes where no longjmp exceptions can be thrown.
---
 gdb/guile/scm-frame.c  | 48 +++++++++++++++++++++++-------------------
 gdb/guile/scm-symbol.c |  4 ++--
 2 files changed, 28 insertions(+), 24 deletions(-)
  

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> The recent patches related to making introduced buld failures when
> trying to build GDB with guile support.  This posed a non-trivial
> problem to fix since guile handles exceptions using longjmp, so we
> couldn't just blindly throw frame_info_ptr everywhere.
>
> The fix makes minimal use of the smart pointer wrapper only using it in
> function calls or in scopes where no longjmp exceptions can be thrown.
> ---
>  gdb/guile/scm-frame.c  | 48 +++++++++++++++++++++++-------------------
>  gdb/guile/scm-symbol.c |  4 ++--
>  2 files changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
> index 159603b8008..51e2ecd9808 100644
> --- a/gdb/guile/scm-frame.c
> +++ b/gdb/guile/scm-frame.c
> @@ -134,7 +134,7 @@ frscm_eq_frame_smob (const void *ap, const void *bp)
>    const frame_smob *a = (const frame_smob *) ap;
>    const frame_smob *b = (const frame_smob *) bp;
>  
> -  return (frame_id_eq (a->frame_id, b->frame_id)
> +  return (a->frame_id == b->frame_id
>  	  && a->inferior == b->inferior
>  	  && a->inferior != NULL);
>  }
> @@ -232,7 +232,7 @@ gdbscm_frame_p (SCM scm)
>     Returns a <gdb:exception> object if there is an error.  */
>  
>  static SCM
> -frscm_scm_from_frame (struct frame_info *frame, struct inferior *inferior)
> +frscm_scm_from_frame (frame_info_ptr frame, struct inferior *inferior)
>  {
>    frame_smob *f_smob, f_smob_for_lookup;
>    SCM f_scm;
> @@ -294,7 +294,11 @@ static SCM
>  frscm_scm_from_frame_unsafe (struct frame_info *frame,
>  			     struct inferior *inferior)
>  {
> -  SCM f_scm = frscm_scm_from_frame (frame, inferior);
> +  SCM f_scm;
> +  {
> +    frame_info_ptr smart_frame(frame);

Missing space after 'smart_frame' here.

> +    f_scm = frscm_scm_from_frame (smart_frame, inferior);
> +  }
>  
>    if (gdbscm_is_exception (f_scm))
>      gdbscm_throw (f_scm);
> @@ -362,16 +366,15 @@ frscm_get_frame_smob_arg_unsafe (SCM self, int arg_pos, const char *func_name)
>  struct frame_info *
>  frscm_frame_smob_to_frame (frame_smob *f_smob)
>  {
> -  struct frame_info *frame;
> +  frame_info_ptr frame = frame_find_by_id (f_smob->frame_id);
>  
> -  frame = frame_find_by_id (f_smob->frame_id);
>    if (frame == NULL)
>      return NULL;
>  
>    if (f_smob->frame_id_is_next)
>      frame = get_prev_frame (frame);
>  
> -  return frame;
> +  return frame.get ();
>  }
>  
>  
> @@ -423,7 +426,7 @@ gdbscm_frame_name (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	name = find_frame_funname (frame, &lang, NULL);
> +	name = find_frame_funname (frame_info_ptr (frame), &lang, NULL);
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -462,7 +465,7 @@ gdbscm_frame_type (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	type = get_frame_type (frame);
> +	type = get_frame_type (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -539,7 +542,7 @@ gdbscm_frame_unwind_stop_reason (SCM self)
>  				   _("<gdb:frame>"));
>      }
>  
> -  stop_reason = get_frame_unwind_stop_reason (frame);
> +  stop_reason = get_frame_unwind_stop_reason (frame_info_ptr (frame));
>  
>    return scm_from_int (stop_reason);
>  }
> @@ -561,7 +564,7 @@ gdbscm_frame_pc (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	pc = get_frame_pc (frame);
> +	pc = get_frame_pc (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -595,7 +598,7 @@ gdbscm_frame_block (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	block = get_frame_block (frame, NULL);
> +	block = get_frame_block (frame_info_ptr (frame), NULL);
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -647,7 +650,7 @@ gdbscm_frame_function (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	sym = find_pc_function (get_frame_address_in_block (frame));
> +	sym = find_pc_function (get_frame_address_in_block (frame_info_ptr (frame)));

I think this line is too long.  Maybe split the frame_info_ptr object
into a separate local within the if block?

>      }
>    catch (const gdb_exception &except)
>      {
> @@ -685,7 +688,7 @@ gdbscm_frame_older (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	prev = get_prev_frame (frame);
> +	prev = get_prev_frame (frame_info_ptr (frame)).get ();
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -723,7 +726,7 @@ gdbscm_frame_newer (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	next = get_next_frame (frame);
> +	next = get_next_frame (frame_info_ptr (frame)).get ();
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -760,7 +763,7 @@ gdbscm_frame_sal (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	sal = find_frame_sal (frame);
> +	sal = find_frame_sal (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -801,11 +804,12 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame)
>  	{
> -	  regnum = user_reg_map_name_to_regnum (get_frame_arch (frame),
> +	  frame_info_ptr p_frame (frame);
> +	  regnum = user_reg_map_name_to_regnum (get_frame_arch (p_frame),
>  						register_str,
>  						strlen (register_str));
>  	  if (regnum >= 0)
> -	    value = value_of_register (regnum, frame);
> +	    value = value_of_register (regnum, p_frame);
>  	}
>      }
>    catch (const gdb_exception &ex)
> @@ -905,7 +909,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
>  	    struct block_symbol lookup_sym;
>  
>  	    if (block == NULL)
> -	      block = get_frame_block (frame, NULL);
> +	      block = get_frame_block (frame_info_ptr (frame), NULL);
>  	    lookup_sym = lookup_symbol (var_name.get (), block, VAR_DOMAIN,
>  					NULL);
>  	    var = lookup_sym.symbol;
> @@ -932,7 +936,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
>  
>    try
>      {
> -      value = read_var_value (var, block, frame);
> +      value = read_var_value (var, block, frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -959,7 +963,7 @@ gdbscm_frame_select (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	select_frame (frame);
> +	select_frame (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -987,7 +991,7 @@ gdbscm_newest_frame (void)
>    gdbscm_gdb_exception exc {};
>    try
>      {
> -      frame = get_current_frame ();
> +      frame = get_current_frame ().get ();
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -1009,7 +1013,7 @@ gdbscm_selected_frame (void)
>    gdbscm_gdb_exception exc {};
>    try
>      {
> -      frame = get_selected_frame (_("No frame is currently selected"));
> +      frame = get_selected_frame (_("No frame is currently selected")).get ();
>      }
>    catch (const gdb_exception &except)
>      {
> diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
> index 4344a633334..5f1d03419d3 100644
> --- a/gdb/guile/scm-symbol.c
> +++ b/gdb/guile/scm-symbol.c
> @@ -551,7 +551,7 @@ gdbscm_symbol_value (SCM self, SCM rest)
>  	 was found, so we have no block to pass to read_var_value.  This will
>  	 yield an incorrect value when symbol is not local to FRAME_INFO (this
>  	 can happen with nested functions).  */
> -      value = read_var_value (symbol, NULL, frame_info);
> +      value = read_var_value (symbol, NULL, frame_info_ptr (frame_info));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -598,11 +598,11 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
>      }
>    else
>      {
> -      struct frame_info *selected_frame;
>  
>        gdbscm_gdb_exception exc {};
>        try
>  	{
> +	  frame_info_ptr selected_frame;
>  	  selected_frame = get_selected_frame (_("no frame selected"));

You should combine the declaration and the assignment of selected_frame.

Approved with those fixes.

Thanks,
Andrew


>  	  block = get_frame_block (selected_frame, NULL);
>  	}
> -- 
> 2.37.3
  

If I catch you before you push, do you think you could write a better
title for the commit, I'd suggest maybe:

  gdb/guile: fix frame_info_ptr related build issues

which is more descriptive when looking through the commit history.

Thanks,
Andrew

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> The recent patches related to making introduced buld failures when
> trying to build GDB with guile support.  This posed a non-trivial
> problem to fix since guile handles exceptions using longjmp, so we
> couldn't just blindly throw frame_info_ptr everywhere.
>
> The fix makes minimal use of the smart pointer wrapper only using it in
> function calls or in scopes where no longjmp exceptions can be thrown.
> ---
>  gdb/guile/scm-frame.c  | 48 +++++++++++++++++++++++-------------------
>  gdb/guile/scm-symbol.c |  4 ++--
>  2 files changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
> index 159603b8008..51e2ecd9808 100644
> --- a/gdb/guile/scm-frame.c
> +++ b/gdb/guile/scm-frame.c
> @@ -134,7 +134,7 @@ frscm_eq_frame_smob (const void *ap, const void *bp)
>    const frame_smob *a = (const frame_smob *) ap;
>    const frame_smob *b = (const frame_smob *) bp;
>  
> -  return (frame_id_eq (a->frame_id, b->frame_id)
> +  return (a->frame_id == b->frame_id
>  	  && a->inferior == b->inferior
>  	  && a->inferior != NULL);
>  }
> @@ -232,7 +232,7 @@ gdbscm_frame_p (SCM scm)
>     Returns a <gdb:exception> object if there is an error.  */
>  
>  static SCM
> -frscm_scm_from_frame (struct frame_info *frame, struct inferior *inferior)
> +frscm_scm_from_frame (frame_info_ptr frame, struct inferior *inferior)
>  {
>    frame_smob *f_smob, f_smob_for_lookup;
>    SCM f_scm;
> @@ -294,7 +294,11 @@ static SCM
>  frscm_scm_from_frame_unsafe (struct frame_info *frame,
>  			     struct inferior *inferior)
>  {
> -  SCM f_scm = frscm_scm_from_frame (frame, inferior);
> +  SCM f_scm;
> +  {
> +    frame_info_ptr smart_frame(frame);
> +    f_scm = frscm_scm_from_frame (smart_frame, inferior);
> +  }
>  
>    if (gdbscm_is_exception (f_scm))
>      gdbscm_throw (f_scm);
> @@ -362,16 +366,15 @@ frscm_get_frame_smob_arg_unsafe (SCM self, int arg_pos, const char *func_name)
>  struct frame_info *
>  frscm_frame_smob_to_frame (frame_smob *f_smob)
>  {
> -  struct frame_info *frame;
> +  frame_info_ptr frame = frame_find_by_id (f_smob->frame_id);
>  
> -  frame = frame_find_by_id (f_smob->frame_id);
>    if (frame == NULL)
>      return NULL;
>  
>    if (f_smob->frame_id_is_next)
>      frame = get_prev_frame (frame);
>  
> -  return frame;
> +  return frame.get ();
>  }
>  
>  
> @@ -423,7 +426,7 @@ gdbscm_frame_name (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	name = find_frame_funname (frame, &lang, NULL);
> +	name = find_frame_funname (frame_info_ptr (frame), &lang, NULL);
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -462,7 +465,7 @@ gdbscm_frame_type (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	type = get_frame_type (frame);
> +	type = get_frame_type (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -539,7 +542,7 @@ gdbscm_frame_unwind_stop_reason (SCM self)
>  				   _("<gdb:frame>"));
>      }
>  
> -  stop_reason = get_frame_unwind_stop_reason (frame);
> +  stop_reason = get_frame_unwind_stop_reason (frame_info_ptr (frame));
>  
>    return scm_from_int (stop_reason);
>  }
> @@ -561,7 +564,7 @@ gdbscm_frame_pc (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	pc = get_frame_pc (frame);
> +	pc = get_frame_pc (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -595,7 +598,7 @@ gdbscm_frame_block (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	block = get_frame_block (frame, NULL);
> +	block = get_frame_block (frame_info_ptr (frame), NULL);
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -647,7 +650,7 @@ gdbscm_frame_function (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	sym = find_pc_function (get_frame_address_in_block (frame));
> +	sym = find_pc_function (get_frame_address_in_block (frame_info_ptr (frame)));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -685,7 +688,7 @@ gdbscm_frame_older (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	prev = get_prev_frame (frame);
> +	prev = get_prev_frame (frame_info_ptr (frame)).get ();
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -723,7 +726,7 @@ gdbscm_frame_newer (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	next = get_next_frame (frame);
> +	next = get_next_frame (frame_info_ptr (frame)).get ();
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -760,7 +763,7 @@ gdbscm_frame_sal (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	sal = find_frame_sal (frame);
> +	sal = find_frame_sal (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -801,11 +804,12 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame)
>  	{
> -	  regnum = user_reg_map_name_to_regnum (get_frame_arch (frame),
> +	  frame_info_ptr p_frame (frame);
> +	  regnum = user_reg_map_name_to_regnum (get_frame_arch (p_frame),
>  						register_str,
>  						strlen (register_str));
>  	  if (regnum >= 0)
> -	    value = value_of_register (regnum, frame);
> +	    value = value_of_register (regnum, p_frame);
>  	}
>      }
>    catch (const gdb_exception &ex)
> @@ -905,7 +909,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
>  	    struct block_symbol lookup_sym;
>  
>  	    if (block == NULL)
> -	      block = get_frame_block (frame, NULL);
> +	      block = get_frame_block (frame_info_ptr (frame), NULL);
>  	    lookup_sym = lookup_symbol (var_name.get (), block, VAR_DOMAIN,
>  					NULL);
>  	    var = lookup_sym.symbol;
> @@ -932,7 +936,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
>  
>    try
>      {
> -      value = read_var_value (var, block, frame);
> +      value = read_var_value (var, block, frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -959,7 +963,7 @@ gdbscm_frame_select (SCM self)
>      {
>        frame = frscm_frame_smob_to_frame (f_smob);
>        if (frame != NULL)
> -	select_frame (frame);
> +	select_frame (frame_info_ptr (frame));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -987,7 +991,7 @@ gdbscm_newest_frame (void)
>    gdbscm_gdb_exception exc {};
>    try
>      {
> -      frame = get_current_frame ();
> +      frame = get_current_frame ().get ();
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -1009,7 +1013,7 @@ gdbscm_selected_frame (void)
>    gdbscm_gdb_exception exc {};
>    try
>      {
> -      frame = get_selected_frame (_("No frame is currently selected"));
> +      frame = get_selected_frame (_("No frame is currently selected")).get ();
>      }
>    catch (const gdb_exception &except)
>      {
> diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
> index 4344a633334..5f1d03419d3 100644
> --- a/gdb/guile/scm-symbol.c
> +++ b/gdb/guile/scm-symbol.c
> @@ -551,7 +551,7 @@ gdbscm_symbol_value (SCM self, SCM rest)
>  	 was found, so we have no block to pass to read_var_value.  This will
>  	 yield an incorrect value when symbol is not local to FRAME_INFO (this
>  	 can happen with nested functions).  */
> -      value = read_var_value (symbol, NULL, frame_info);
> +      value = read_var_value (symbol, NULL, frame_info_ptr (frame_info));
>      }
>    catch (const gdb_exception &except)
>      {
> @@ -598,11 +598,11 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
>      }
>    else
>      {
> -      struct frame_info *selected_frame;
>  
>        gdbscm_gdb_exception exc {};
>        try
>  	{
> +	  frame_info_ptr selected_frame;
>  	  selected_frame = get_selected_frame (_("no frame selected"));
>  	  block = get_frame_block (selected_frame, NULL);
>  	}
> -- 
> 2.37.3
  

Ignore me.  I started reviewing Bruno's patch just before Tom posted his
version[1].  And it looks like we're running with Tom's patch.

Thanks,
Andrew

[1]  https://sourceware.org/pipermail/gdb-patches/2022-October/192521.html

Andrew Burgess <aburgess@redhat.com> writes:

> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> The recent patches related to making introduced buld failures when
>> trying to build GDB with guile support.  This posed a non-trivial
>> problem to fix since guile handles exceptions using longjmp, so we
>> couldn't just blindly throw frame_info_ptr everywhere.
>>
>> The fix makes minimal use of the smart pointer wrapper only using it in
>> function calls or in scopes where no longjmp exceptions can be thrown.
>> ---
>>  gdb/guile/scm-frame.c  | 48 +++++++++++++++++++++++-------------------
>>  gdb/guile/scm-symbol.c |  4 ++--
>>  2 files changed, 28 insertions(+), 24 deletions(-)
>>
>> diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
>> index 159603b8008..51e2ecd9808 100644
>> --- a/gdb/guile/scm-frame.c
>> +++ b/gdb/guile/scm-frame.c
>> @@ -134,7 +134,7 @@ frscm_eq_frame_smob (const void *ap, const void *bp)
>>    const frame_smob *a = (const frame_smob *) ap;
>>    const frame_smob *b = (const frame_smob *) bp;
>>  
>> -  return (frame_id_eq (a->frame_id, b->frame_id)
>> +  return (a->frame_id == b->frame_id
>>  	  && a->inferior == b->inferior
>>  	  && a->inferior != NULL);
>>  }
>> @@ -232,7 +232,7 @@ gdbscm_frame_p (SCM scm)
>>     Returns a <gdb:exception> object if there is an error.  */
>>  
>>  static SCM
>> -frscm_scm_from_frame (struct frame_info *frame, struct inferior *inferior)
>> +frscm_scm_from_frame (frame_info_ptr frame, struct inferior *inferior)
>>  {
>>    frame_smob *f_smob, f_smob_for_lookup;
>>    SCM f_scm;
>> @@ -294,7 +294,11 @@ static SCM
>>  frscm_scm_from_frame_unsafe (struct frame_info *frame,
>>  			     struct inferior *inferior)
>>  {
>> -  SCM f_scm = frscm_scm_from_frame (frame, inferior);
>> +  SCM f_scm;
>> +  {
>> +    frame_info_ptr smart_frame(frame);
>
> Missing space after 'smart_frame' here.
>
>> +    f_scm = frscm_scm_from_frame (smart_frame, inferior);
>> +  }
>>  
>>    if (gdbscm_is_exception (f_scm))
>>      gdbscm_throw (f_scm);
>> @@ -362,16 +366,15 @@ frscm_get_frame_smob_arg_unsafe (SCM self, int arg_pos, const char *func_name)
>>  struct frame_info *
>>  frscm_frame_smob_to_frame (frame_smob *f_smob)
>>  {
>> -  struct frame_info *frame;
>> +  frame_info_ptr frame = frame_find_by_id (f_smob->frame_id);
>>  
>> -  frame = frame_find_by_id (f_smob->frame_id);
>>    if (frame == NULL)
>>      return NULL;
>>  
>>    if (f_smob->frame_id_is_next)
>>      frame = get_prev_frame (frame);
>>  
>> -  return frame;
>> +  return frame.get ();
>>  }
>>  
>>  
>> @@ -423,7 +426,7 @@ gdbscm_frame_name (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	name = find_frame_funname (frame, &lang, NULL);
>> +	name = find_frame_funname (frame_info_ptr (frame), &lang, NULL);
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -462,7 +465,7 @@ gdbscm_frame_type (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	type = get_frame_type (frame);
>> +	type = get_frame_type (frame_info_ptr (frame));
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -539,7 +542,7 @@ gdbscm_frame_unwind_stop_reason (SCM self)
>>  				   _("<gdb:frame>"));
>>      }
>>  
>> -  stop_reason = get_frame_unwind_stop_reason (frame);
>> +  stop_reason = get_frame_unwind_stop_reason (frame_info_ptr (frame));
>>  
>>    return scm_from_int (stop_reason);
>>  }
>> @@ -561,7 +564,7 @@ gdbscm_frame_pc (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	pc = get_frame_pc (frame);
>> +	pc = get_frame_pc (frame_info_ptr (frame));
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -595,7 +598,7 @@ gdbscm_frame_block (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	block = get_frame_block (frame, NULL);
>> +	block = get_frame_block (frame_info_ptr (frame), NULL);
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -647,7 +650,7 @@ gdbscm_frame_function (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	sym = find_pc_function (get_frame_address_in_block (frame));
>> +	sym = find_pc_function (get_frame_address_in_block (frame_info_ptr (frame)));
>
> I think this line is too long.  Maybe split the frame_info_ptr object
> into a separate local within the if block?
>
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -685,7 +688,7 @@ gdbscm_frame_older (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	prev = get_prev_frame (frame);
>> +	prev = get_prev_frame (frame_info_ptr (frame)).get ();
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -723,7 +726,7 @@ gdbscm_frame_newer (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	next = get_next_frame (frame);
>> +	next = get_next_frame (frame_info_ptr (frame)).get ();
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -760,7 +763,7 @@ gdbscm_frame_sal (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	sal = find_frame_sal (frame);
>> +	sal = find_frame_sal (frame_info_ptr (frame));
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -801,11 +804,12 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame)
>>  	{
>> -	  regnum = user_reg_map_name_to_regnum (get_frame_arch (frame),
>> +	  frame_info_ptr p_frame (frame);
>> +	  regnum = user_reg_map_name_to_regnum (get_frame_arch (p_frame),
>>  						register_str,
>>  						strlen (register_str));
>>  	  if (regnum >= 0)
>> -	    value = value_of_register (regnum, frame);
>> +	    value = value_of_register (regnum, p_frame);
>>  	}
>>      }
>>    catch (const gdb_exception &ex)
>> @@ -905,7 +909,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
>>  	    struct block_symbol lookup_sym;
>>  
>>  	    if (block == NULL)
>> -	      block = get_frame_block (frame, NULL);
>> +	      block = get_frame_block (frame_info_ptr (frame), NULL);
>>  	    lookup_sym = lookup_symbol (var_name.get (), block, VAR_DOMAIN,
>>  					NULL);
>>  	    var = lookup_sym.symbol;
>> @@ -932,7 +936,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
>>  
>>    try
>>      {
>> -      value = read_var_value (var, block, frame);
>> +      value = read_var_value (var, block, frame_info_ptr (frame));
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -959,7 +963,7 @@ gdbscm_frame_select (SCM self)
>>      {
>>        frame = frscm_frame_smob_to_frame (f_smob);
>>        if (frame != NULL)
>> -	select_frame (frame);
>> +	select_frame (frame_info_ptr (frame));
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -987,7 +991,7 @@ gdbscm_newest_frame (void)
>>    gdbscm_gdb_exception exc {};
>>    try
>>      {
>> -      frame = get_current_frame ();
>> +      frame = get_current_frame ().get ();
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -1009,7 +1013,7 @@ gdbscm_selected_frame (void)
>>    gdbscm_gdb_exception exc {};
>>    try
>>      {
>> -      frame = get_selected_frame (_("No frame is currently selected"));
>> +      frame = get_selected_frame (_("No frame is currently selected")).get ();
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
>> index 4344a633334..5f1d03419d3 100644
>> --- a/gdb/guile/scm-symbol.c
>> +++ b/gdb/guile/scm-symbol.c
>> @@ -551,7 +551,7 @@ gdbscm_symbol_value (SCM self, SCM rest)
>>  	 was found, so we have no block to pass to read_var_value.  This will
>>  	 yield an incorrect value when symbol is not local to FRAME_INFO (this
>>  	 can happen with nested functions).  */
>> -      value = read_var_value (symbol, NULL, frame_info);
>> +      value = read_var_value (symbol, NULL, frame_info_ptr (frame_info));
>>      }
>>    catch (const gdb_exception &except)
>>      {
>> @@ -598,11 +598,11 @@ gdbscm_lookup_symbol (SCM name_scm, SCM rest)
>>      }
>>    else
>>      {
>> -      struct frame_info *selected_frame;
>>  
>>        gdbscm_gdb_exception exc {};
>>        try
>>  	{
>> +	  frame_info_ptr selected_frame;
>>  	  selected_frame = get_selected_frame (_("no frame selected"));
>
> You should combine the declaration and the assignment of selected_frame.
>
> Approved with those fixes.
>
> Thanks,
> Andrew
>
>
>>  	  block = get_frame_block (selected_frame, NULL);
>>  	}
>> -- 
>> 2.37.3