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

[Hui Zhu <teawater at gmail dot com>]

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
>

Attachment: prec_software_single_step.txt
Description: Text document