Re: [RFA] Reverse Debugging, 4/5

[Joel Brobecker <brobecker at adacore dot com>]

> 	* breakpoint.c (breakpoint_silence): New function.
> 	* infcmd.c (finish_command): Check for reverse exec direction.

You're going to hate me, now :).  This is another instance where
I think we can break the code a little differently:

  1. finish_command_backwards (I would have prefered
     "reverse_finish_command but :-P)

  2. finish_command_forward

  3. finish_command:
     {
       [do all the stuff about checking for args, etc]
       if (target_get_exec_dir () == EXEC_REVERSE)
         finish_command_backwards ()
       else
         finish_command_forward ()
     }

That way, we just split the finish_command code into two parts
without moving some of the code, and it's clear that the two paths
are completely distinct.  The "branch-off" approach (that we used
for Ada but that I'm trying to avoid like the plague now) does obscure
the structure of your program.

> +void
> +breakpoint_silence (struct breakpoint *b)
> +{
> +  /* Silence the breakpoint.  */
> +  b->silent = 1;

Minor nit: This name brings little meaning when I see it being called.
Can we change it to "make_breakpoint_silent"?  That way, the comment
in the body becomes useless and can be removed.

> +  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.");

Internal error? I understand that it should probably never happen
in this context, but how about making it a simple error instead.
If we trip this check, it's true that something went wrong, but
let's just abort the command and let the user try to continue
rather than asking the user whether we should abort the whole
session.

> +  /* TODO: Let's not worry about async until later.  */

Should we add a check now, though, and error out if async was requested?

> +      /* (Kludgy way of letting wait_for_inferior know...) */
> +      tp->step_range_start = tp->step_range_end = 1;

AARGH! More special meaning to these addresses. We really ought to
clean these up and put some specific flags in the structure, one day.
I don't know why we're trying so hard to resume these fields.

-- 
Joel