Re: [RFC] Add support of software single step to process record

[Joel Brobecker <brobecker@adacore.com>]

Hui:

> >2009-12-18  Hui Zhu  <teawater@gmail.com>
> >
> >	* breakpoint.c (inserted_single_step_breakpoint_p): New
> >	function.
> >	* breakpoint.h (inserted_single_step_breakpoint_p): Extern.
> >	* record.c (record_resume): Add code for software single step.
> >	(record_wait): Ditto.

I understand Michael's answer as approval.  I do not see any problem
with it, but my knowledge of the target stack in the resume/wait area
is pretty sketchy.

Just a stylistic comment on the patch:

> >+/* Check if the breakpoints used for software single stepping
> >inserted or not.  */

Formatting and missing "are".

/* Check if the breakpoints used for software single stepping
   are inserted or not.  */

> >+int
> >+inserted_single_step_breakpoint_p (void)

Can you rename this function to:

        single_step_breakpoints_inserted

The "inserted" already conveys the idea of a condition/predicate,
so the _p is superfluous in this case.

I'm also a little worried about the code adding calls to functions
that in essence return a global variable. For instance, you are
introducing calls to get_current_frame or current_gdbarch().
Ulrich is one of our specialists in this area, whereas I'm not sure,
but I am wondering if we are introducing any latent issue by using
these routines...

-- 
Joel