Re: [PATCH] Replace deprecated_target_wait_hook by an observer

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Patrick Monnerat <patrick@monnerat.net> [2021-08-25 15:30:28 +0200]:

> 
> On 8/24/21 6:14 PM, Andrew Burgess wrote:
> > * Patrick Monnerat via Gdb-patches <gdb-patches@sourceware.org> [2021-08-22 18:42:56 +0200]:
> > 
> > > +DEFINE_OBSERVABLE (waiting_for_target);
> > Given we already have events 'target_changed' and 'target_resumed', I
> > wonder if it would be more consistent to name this event 'target_wait'?
> > 
> Yes, it's possible. Here are the two reasons why I did not name this
> observer 'target_wait':
> 
> 1) Because the observer is not supposed to wait by itself, I fear it will be
> a source of confusion.
> 
> 2) As a good old C programmer, I still have some reluctance naming an object
> as a global procedure. Not being declared in the same namespace though!
> 
> Comments are welcome!

How about 'target_waiting' then?  This seems more inline with the
existing naming, seems to indicate that the target _is_ waiting, not
the that observer _should_ wait (so avoiding #1), and is a new name
(so avoiding #2).

> 
> > > -  return target->wait (ptid, status, options);
> > > +  gdb::observers::waiting_for_target.notify (true, ptid);
> > > +  event_ptid = target->wait (ptid, status, options);
> > > +  gdb::observers::waiting_for_target.notify (false, event_ptid);
> > > +  return event_ptid;
> > I would be tempted to wrap this notification inside an RAII class,
> > then we will be guaranteed to send the second notification, even in
> > the event that the wait call exits via an exception.
> 
> That would make sense, but in case we have an exception, event_ptid is not
> known. How would you handle it? pass it as null_ptid?

Yeah I guess that would make sense.

Thanks,
Andrew

> 
> Thanks for review,
> 
> Patrick
>