Hi guys,

I make a new patch according to your comment.

Shuchang, please help me try this patch?

Joel, I remove the current_gdbarch() from this patch, but for
get_current_frame, I cannot find any function that fit for replace it.
 Could you help me with it?

Thanks,
Hui

On Sun, Dec 20, 2009 at 21:30, Joel Brobecker <brobecker@adacore.com> wrote:
> 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
>