Re: [PATCH] Fix build breakage from recent patches

[Andrew Burgess <aburgess@redhat.com>]

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 ();
>  }
>  
>  \f
> @@ -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