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

[Gary Benson <gbenson@redhat.com>]

Doug Evans wrote:
> 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.
> [...]
> > +/* See target/target.h.  */
> > +
> > +void
> > +target_continue_ptid (ptid_t ptid)
> > +{
> > +  target_resume (ptid, 0, GDB_SIGNAL_0);
> > +}
> 
> 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.

I don't know the answer to this, I just moved the code from one
place to the next :)  Possibly it's because (I think) under the
hood target_stop_ptid sends a SIGSTOP to the inferior, so you
don't want to restart it with that signal queued.

The comment for target_continue_ptid says:

> > +/* Restart a target that was previously stopped by target_stop_ptid.
> > +   This function must be provided by the client.  */

That implies to me "don't use this for targets not previously
stopped by target_stop_ptid".

Maybe someone more familiar with this code could elaborate?

Thanks,
Gary

-- 
http://gbenson.net/