Re: [RFC] gdbscm_wrap, eliminate GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS (Re: [RFA 2/4] Change gdbscm_exception_message_to_string to return a unique_xmalloc_ptr)

[Tom Tromey <tom@tromey.com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I gave this a try today.  How about this?

Pedro> +/* Use this to wrap a callable to throw the appropriate Scheme
Pedro> +   exception if the callable throws a GDB error.  ARGS are forwarded
Pedro> +   to FUNC.  Returns the result of FUNC, unless FUNC returns a Scheme
Pedro> +   exception, in which case that exception is thrown.  Note that while
Pedro> +   the callable is free is use objects of types with destructors,

Typo - "free to use objects".

Pedro> +++ b/gdb/guile/scm-frame.c
Pedro> @@ -783,13 +783,11 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
Pedro>    char *register_str;
Pedro>    struct value *value = NULL;
Pedro>    struct frame_info *frame = NULL;
Pedro> -  struct cleanup *cleanup;
Pedro>    frame_smob *f_smob;
 
Pedro>    f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
Pedro>    gdbscm_parse_function_args (FUNC_NAME, SCM_ARG2, NULL, "s",
Pedro>  			      register_scm, &register_str);
Pedro> -  cleanup = make_cleanup (xfree, register_str);
 
Pedro>    TRY
Pedro>      {
Pedro> @@ -811,7 +809,7 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
Pedro>      }
Pedro>    END_CATCH
 
Pedro> -  do_cleanups (cleanup);
Pedro> +  xfree (register_str);

I think this will leak register_str if anything throws.  Maybe the xfree
should be duplicated in the catch block, or maybe the whole try/catch
should be replaced with a call to gdbscm_wrap (where the callback would
take ownership of register_str).

This seems like a latent bug here as well - I think the code should be using
GDBSCM_HANDLE_GDB_EXCEPTION_WITH_CLEANUPS here.


Otherwise this all looks good to me.  Thanks for doing this, guile was
one of the larger cleanup holdouts

Tom