Re: [RFA] Reverse Debugging, 3/5

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 06 October 2008 21:51:39, Michael Snyder wrote:
> Pedro Alves wrote:
> > Hi Michael,
> > 
> > Shouldn't failing to find ones
> > direction always be an error (hence an error call from inside
> > target_get_execution_direction, or something alike).
> 
> Targets that don't implement reverse return EXEC_ERROR,
> rather than EXEC_FORWARD.  It was an early interface
> design decision, and I'm not sure if I can remember the
> justification after over 2 years, but I made it
> consciously -- it seemed to simplify things.
> 

...  Okay.  If nobody else remembers why, and I throw a
later patch at you to change this, please don't be mad
at me.  :-)

> 
> >> +        keep going back to the call point).  */
> >> +      if (stop_pc == ecs->event_thread->step_range_start &&
> >> +         stop_pc != ecs->stop_func_start &&
> >> +         target_get_execution_direction () == EXEC_REVERSE)
> >> +       {
> >> +         ecs->event_thread->stop_step = 1;
> >> +         print_stop_reason (END_STEPPING_RANGE, 0);
> >> +         stop_stepping (ecs);
> >> +       }
> > 
> >> +      else
> >> +       {
> >> +         keep_going (ecs);
> >> +       }
> > 
> > Unneeded braces.
> 
> Don't you think it's more readable if the if block
> and the else block match?

Not really, and it's what the GDB/GNU coding standards prefer...

"
For the body of the function, our recommended style looks like this:

     if (x < foo (y, z))
       haha = bar[4] + 5;
     else
       {
         while (z)
           {
             haha += foo (z, z);
             z--;
           }
         return ++x + bar ();
       }
"


> >> +       }
> >> +      else
> >> +       {
> >> +         /* Set a breakpoint at callee's return address (the address
> >> +            at which the caller will resume).  */
> >> +         insert_step_resume_breakpoint_at_caller (get_current_frame ());
> >> +       }
> > 
> > Unneeded braces.
> 
> Oh come on -- I know they're syntactic null, but
> they serve to keep the comment together with the
> code it refers to.
> 

I'm not going to argue about these issues, but, personally, and to
stick with the standard, I do things like this a lot:

      if (foo)
        {
          /* Set a breakpoint at callee's return address (the address
             at which the caller will resume).  */
          goo ();
          bar ();
        }
      else
        /* Set a breakpoint at callee's return address (the address
           at which the caller will resume).  */
        insert_step_resume_breakpoint_at_caller (get_current_frame ());

> >> +      /* Else just reset the step range and keep going.
> >> +        No step-resume breakpoint, they don't work for
> >> +        epilogues, which can have multiple entry paths.  */
> >> +      ecs->event_thread->step_range_start = stop_func_sal.pc;
> >> +      ecs->event_thread->step_range_end   = stop_func_sal.end;
> > 
> > Somethings fishy with the whitespace.     ^
> 
> I just like things to line up when possible!
> ;-)

To me, visual vertical aligment is more distracting than good.  It
distract me from the right -> left assignment flow.  But, that's
just me.  I'm not going to make a bid deal out of it.

-- 
Pedro Alves