Re: [RFA] Resubmit reverse debugging [4/5]

[Michael Snyder <msnyder at vmware dot com>]

Pedro Alves wrote:

> > +/* finish_backward -- helper function for finish_command.  */
> > +
> > +static void
> > +finish_backward (struct symbol *function, struct thread_info *tp)
> > +{
> > +  struct symtab_and_line sal;
> > +  struct breakpoint *breakpoint;
> > +  struct cleanup *old_chain;
> > +  CORE_ADDR func_addr;
> > +  int back_up;
> > +
> > +  if (find_pc_partial_function (get_frame_pc (get_current_frame ()),
> > +                               NULL, &func_addr, NULL) == 0)
> > +    internal_error (__FILE__, __LINE__,
> > +                   _("Finish: couldn't find function."));
> > +
>
> Still internal_error?

Sorry, it's an artifact of the fact that I've been on a
fork for so long.  When I copied this code from finish_command,
the code that I copied had a similar call to internal_error.

In fact, finish_command_continuation still does.
In fact, it's the same call that used to be in "finish_command".

So what should it be?  Just "error"?

> > +  sal = find_pc_line (func_addr, 0);
> > +
> > +  /* TODO: Let's not worry about async until later.  */
> > +
>
> Should be an error here instead of on finish_command ...
> (keep reading)

OK, I forgot to remove the comment...

But I put the error in finish_command, because that is
where all of the necessary information is available.

In order to put the error here, I would have to add
more function parameters and pass more information.

I think I understand that you think it would be more "local"
to put the error here -- but is it worth it if it makes us
add complexity?

finish_command already tests a number of things, including
whether we are async and (now) whether we are reverse, and
contains a number of error calls already.