Re: [PATCH v2] guile: stop procedures on invalid breakpoints

[Simon Marchi <simon.marchi@polymtl.ca>]

> diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
> index 4ff197e48a4..3c741d712b7 100644
> --- a/gdb/guile/scm-breakpoint.c
> +++ b/gdb/guile/scm-breakpoint.c
> @@ -326,6 +326,26 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
>  
>    return bp_smob;
>  }
> +
> +/* Returns the breakpoint smob in SELF, verifying it's either valid or
> +   originates from Scheme.
> +   Throws an exception if SELF is not a <gdb:breakpoint> object,
> +   or is invalid and not allocated from Scheme.  */
> +
> +static breakpoint_smob *
> +bpscm_get_valid_or_scm_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
> +						   const char *func_name)
> +{
> +  breakpoint_smob *bp_smob
> +    = bpscm_get_breakpoint_smob_arg_unsafe (self, arg_pos, func_name);
> +
> +  if (!bpscm_is_valid (bp_smob) && !bp_smob->is_scheme_bkpt)
> +    gdbscm_invalid_object_error (func_name, arg_pos, self,
> +				 _("<gdb:breakpoint>"));
> +
> +
> +  return bp_smob;

Remove extra empty line above.

> +}
>  \f
>  /* Breakpoint methods.  */
>  
> @@ -914,7 +934,8 @@ static SCM
>  gdbscm_breakpoint_stop (SCM self)
>  {
>    breakpoint_smob *bp_smob
> -    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> +    = bpscm_get_valid_or_scm_breakpoint_smob_arg_unsafe (self, SCM_ARG1,
> +							 FUNC_NAME);
>  
>    return bp_smob->stop;
>  }
> @@ -926,33 +947,36 @@ static SCM
>  gdbscm_set_breakpoint_stop_x (SCM self, SCM newvalue)
>  {
>    breakpoint_smob *bp_smob
> -    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> -  const struct extension_language_defn *extlang = NULL;
> +    = bpscm_get_valid_or_scm_breakpoint_smob_arg_unsafe (self, SCM_ARG1,
> +							 FUNC_NAME);
>  
>    SCM_ASSERT_TYPE (gdbscm_is_procedure (newvalue)
>  		   || gdbscm_is_false (newvalue),
>  		   newvalue, SCM_ARG2, FUNC_NAME,
>  		   _("procedure or #f"));
>  
> -  if (bp_smob->bp->cond_string != NULL)
> -    extlang = get_ext_lang_defn (EXT_LANG_GDB);
> -  if (extlang == NULL)
> -    extlang = get_breakpoint_cond_ext_lang (bp_smob->bp, EXT_LANG_GUILE);
> -  if (extlang != NULL)
> -    {
> -      char *error_text
> -	= xstrprintf (_("Only one stop condition allowed.  There is"
> -			" currently a %s stop condition defined for"
> -			" this breakpoint."),
> -		      ext_lang_capitalized_name (extlang));
> -
> -      scm_dynwind_begin ((scm_t_dynwind_flags) 0);
> -      gdbscm_dynwind_xfree (error_text);
> -      gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self, error_text);
> -      /* The following line, while unnecessary, is present for completeness
> -	 sake.  */
> -      scm_dynwind_end ();
> -    }
> +  if (bp_smob->bp != nullptr) {
> +    const struct extension_language_defn *extlang = nullptr;
> +    if (bp_smob->bp->cond_string != nullptr)
> +      extlang = get_ext_lang_defn (EXT_LANG_GDB);
> +    if (extlang == nullptr)
> +      extlang = get_breakpoint_cond_ext_lang (bp_smob->bp, EXT_LANG_GUILE);
> +    if (extlang != nullptr)
> +      {
> +	char *error_text
> +	  = xstrprintf (_("Only one stop condition allowed.  There is"
> +			  " currently a %s stop condition defined for"
> +			  " this breakpoint."),
> +			ext_lang_capitalized_name (extlang));
> +
> +	scm_dynwind_begin ((scm_t_dynwind_flags) 0);
> +	gdbscm_dynwind_xfree (error_text);
> +	gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self, error_text);
> +	/* The following line, while unnecessary, is present for
> +	   completeness sake.  */
> +	scm_dynwind_end ();
> +      }
> +  }

Format the if above with the usual style:

  if (...)
    {
      ...
    }

The patch LGTM with those fixed.  Again, we will be able to merge it
once your copyright assignment is complete, just remind us then.

Simon