Re: Patch to propagate GDB's knowledge of the executing state to frontend

[dje@google.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.