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

Gary Benson
2014-09-11T10:27:05Z

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

--