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

[Michael Snyder <msnyder@vmware.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.