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

Doug Evans
2014-09-12T18:08:47Z

Pedro Alves writes:
> On 09/12/2014 06:38 PM, Doug Evans wrote:
> > Pedro Alves writes:
> > > On 09/12/2014 05:53 PM, Doug Evans wrote:
> > >
> > > > That may be sufficient for me to make this patch checkin-able,
> > > > but before then I'd like to understand how and where
> > > > this function will be used from gdb.
> > >
> > > Can you clarify? I don't have any plan to use this
> > > elsewhere myself. All we're doing is factoring out
> > > the current use behind a common function so that both gdb
> > > and gdbserver can implement it their own way.
> >
> > I don't see the function being used in gdb by this patch set.
> > So, absent further info, the change to gdb/target.c is just adding
> > dead code. I'm assuming that's not the case (*1), but until I
> > understand how it's going to be used in gdb it's not clear to
> > me whether code that uses it will be confusing to read.
>
> It's used in common/agent.c, which is used by gdb as well.
>
> linux-nat.c: agent_run_command (pid, s, strlen (s) + 1);
> linux-nat.c: agent_run_command (pid, s, strlen (s) + 1);
>
> It's because it's used in gdb that the current code, before
> Gary's patch has the #ifdef GDBSERVER #else /* gdb code */ bits.
> He's just moving that code around.

Ah.
I'm liking target_continue_with_no_signal even more.
[assuming one wants to leave the signature as is]

btw, I noticed this in linux-nat.c:

/* Pause all */
target_stop (ptid);

memcpy (s, "qTfSTM", sizeof ("qTfSTM"));
s[sizeof ("qTfSTM")] = 0;

agent_run_command (pid, s, strlen (s) + 1);

And I remembered you saying:

> I just now noticed the elephant in the room -- target_stop is
> asynchronous, doesn't wait for a stop, while and target_stop_ptid
> is synchronous. [...]

If the above code is right, I think a clarifying comment
is required somewhere. It's odd that one can call agent_run_command
when the inferior may or may not be stopped yet.
[Or is there a bug here? - if I'm reading the gdbserver version
correctly it first waits for the inferior to stop]

The function comment for target_stop says nothing one way or the other
about whether it's asynchronous which doesn't help.