This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Patch to propagate GDB's knowledge of the executing state to frontend
- From: dje at google dot com
- To: Yao Qi <yao at codesourcery dot com>
- Cc: ali_anwar <ali_anwar at codesourcery dot com>, <gdb-patches at sourceware dot org>
- Date: Fri, 2 Nov 2012 09:15:14 -0700
- Subject: Re: Patch to propagate GDB's knowledge of the executing state to frontend
- References: <50891E05.7050509@codesourcery.com> <508F719C.2080409@codesourcery.com>
Yao Qi writes:
> On 10/25/2012 07:09 PM, ali_anwar wrote:
> > [...]
> > @@ -1,3 +1,13 @@
> > +2012-10-25 Ali Anwar<ali_anwar@codesourcery.com>
> > +
> > + * infrun.c (handle_inferior_event_stub, regcache_dup_stub):
> > + New functions.
> > + (normal_stop): Change to propagate GDB's knowledge of the
> > + executing state to frontend when not able to fetch registers.
> > + (wait_for_inferior): Chnage to propagate GDB's knowledge of
> ^^^^^^ typo
>
>
> > + the executing state if not able to fetch backtrace once the
> > + step has already occured.
> ^^^^^^^ typo.
>
> In each changelog entry, we'll put 'what do we change' instead of 'why
> do we change in this way'. So this entry can be simplified.
Hi.
I agree with your first sentence, and would add that if such an
explanation is needed, it belongs in the code not the changelog.
[We don't have enough comments in the code explaining *why* things
are the way they are.]
But I'd say that's not the case here, at least for the changelog entries.
Instead, I would remove the leading "Change to", and just say "Propagate ...".
Also, I would add a comment to the code explaining *why* the calls are wrapped
in catch_error (and I would have the comment live at the call to catch_error,
not in the definition of the two new stubs).
One could also say the two new functions also require comments,
but they're pretty simple and hook_stop_stub doesn't have a comment,
so I'd be ok with leaving them out.