Re: [PATCH 3/9 v7] Introduce target_{stop,continue}_ptid

[Doug Evans <dje@google.com>]

Gary Benson writes:
 > This commit introduces two new functions to stop and restart target
 > processes that shared code can use and that clients must implement.
 > It also changes some shared code to use these functions.
 > 
 > The changes in this patch replace the target_stop, target_wait
 > and target_resume parts of this patch I posted on August 1:
 > https://sourceware.org/ml/gdb-patches/2014-08/msg00014.html
 > The remainder of that patch is in patch 2 of this series.
 > 
 > gdb/ChangeLog:
 > 
 > 	* target/target.h (target_stop_ptid, target_continue_ptid):
 > 	Declare.
 > 	* target.c (target_stop_ptid, target_continue_ptid): New
 > 	functions.
 > 	* common/agent.c [!GDBSERVER]: Don't include infrun.h.
 > 	(agent_run_command): Always use target_stop_ptid and
 > 	target_continue_ptid.
 > 
 > gdb/gdbserver/ChangeLog:
 > 
 > 	* target.c (target_stop_ptid, target_continue_ptid): New
 > 	functions.
 > [...]
 > diff --git a/gdb/target.c b/gdb/target.c
 > index 711e7cb..339b1d1 100644
 > --- a/gdb/target.c
 > +++ b/gdb/target.c
 > @@ -3027,6 +3027,31 @@ target_stop (ptid_t ptid)
 >    (*current_target.to_stop) (&current_target, ptid);
 >  }
 >  
 > [...]
 > +/* See target/target.h.  */
 > +
 > +void
 > +target_continue_ptid (ptid_t ptid)
 > +{
 > +  target_resume (ptid, 0, GDB_SIGNAL_0);
 > +}
 > +

Hi.
How come GDB_SIGNAL_0 is used here?
Maybe it's correct, but it's not immediately clear.

The reason I ask is because there are two ways to "continue"
the inferior:
1) resume it where it left off, and if it stopped because
   of a signal then forward on that signal (assuming the
   signal is not "nopass") (GDB_SIGNAL_DEFAULT).
2) Either inject a new signal (GDB_SIGNAL_FOO) or cancel out a previously
   queued signal (GDB_SIGNAL_0).

GDB_SIGNAL_0 is used to resume the target and discarding
any signal that it may have stopped for.
GDB_SIGNAL_DEFAULT is used for (1).

I realize the comments for target_resume say to not pass
GDB_SIGNAL_DEFAULT to it.  But the name "target_continue_ptid"
with no option to choose between (1) and (2)
says to me "do what GDB_SIGNAL_DEFAULT" does.

 > +/* Restart a target that was previously stopped by target_stop_ptid.
 > +   This function must be provided by the client.  */
 > +
 > +extern void target_continue_ptid (ptid_t ptid);
 > +