Re: [RFA] let record_resume fail immediately on error

[Hui Zhu <teawater@gmail.com>]

Hi guys,

What about following change:
Change record_message to record_message (struct regcache *regcache,
enum target_signal signal)

static int
record_message_wrapper_safe(struct regcache *regcache,
		   enum target_signal signal)
{
  struct record_message_args args;

  args.regcache = regcache;
  args.signal = signal;
  return catch_errors (record_message_wrapper, &args, NULL, RETURN_MASK_ALL);
}

static int
record_message_wrapper (void *args)
{
  return record_message (args->regcache, args->signal);
}

record_resume will call record_message.
record_wait will call record_message_wrapper_safe

Do you think it's OK?

Thanks,
Hui


On Thu, Nov 26, 2009 at 01:58, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>
>>> I don't like use TRY_CATCH or catch_errors directly.
>
> Joel> I confess that I don't like catch_errors, because of the need to
> Joel> artificially create a container type that contains all the function
> Joel> parameters.
>
> In this code, we already have the container type and untyped trampoline
> function.
>
> The problem I have with it is that there are direct calls to the untyped
> trampoline function.
>
> I think the general approach for using catch_errors in gdb ought to be:
>
> * Have a properly-typed function that does all the work.
> * If you need catch_errors, introduce a new type to hold the actual
>  arguments, and write an untyped trampoline function.  Then, *only*
>  call this trampoline function via catch_errors.
> * ... However, prefer TRY_CATCH in most cases, because it does not
>  require a new type and is generally safer (though not completely
>  safe).
>
> And FWIW, I think this rule is generally followed in practice.
>
> My reason for the above is that it is generally best to write a
> type-safe program and let the compiler diagnose errors.  We can't do
> this for catch_errors, due to limitations in C, but we can at least
> limit the damage.
>
>
> Also, this patch introduces an argument indicating whether or not to
> catch.  This is bad, because it is confusing, but also particularly bad
> in this case because the actual argument is always a constant.
>
> Tom
>