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

Doug Evans dje@google.com
Fri Sep 12 18:08:00 GMT 2014


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.



More information about the Gdb-patches mailing list