This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Add support of software single step to process record
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Michael Snyder <msnyder at vmware dot com>
- Cc: Hui Zhu <teawater at gmail dot com>, gdb-patches ml <gdb-patches at sourceware dot org>, shuchang zhou <shuchang dot zhou at gmail dot com>, paawan oza <paawan1982 at yahoo dot com>
- Date: Sun, 20 Dec 2009 17:30:09 +0400
- Subject: Re: [RFC] Add support of software single step to process record
- References: <daef60380912180021h5d029e55k7dc4e3f4c8d33b36@mail.gmail.com> <4B2BD97E.1020106@vmware.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