Re: [RFA] Restore leading zeros in remote_thread_alive

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

On Wednesday 22 October 2008 19:05:09, Michael Snyder wrote:
> Pedro Alves wrote:
> 
> > Could you try this out please?  It works here against gdbserver
> > single|multi-process, and sends a `T000000tid' (tid alive) packet when
> > multi-process isn't in effect.
> 
> Thanks for working up the patch -- it works for my target.

Thanks for testing.

> 
> However, I must say that I'm not crazy about the idea that
> we decide whether or not to send leading zeroes based on
> whether the target is multi-process.  Seems orthogonal
> and ad hoc.

I can't think of a valid reason why we'd have this
inconsistency:

 --> Hg p1.2
 ...

 --> vCont;s:p1.2
 <-- T05 thread:p1.2

 -->  qfThreadInfo
 <--  m p1.2
 -->  qfThreadInfo
 <--  m p1.3
 ...

 (others)

But:

 --> T p00000001.00000002

?

Especially since we centralized the thread-id definition for multi-process:

 "In addition, the remote protocol supports a multiprocess feature
 in which the thread-id syntax is extended to optionally include both
 process and thread ID fields, as `ppid.tid'. The pid (process) and
 tid (thread) components each have the format described above: a
 positive number with target-specific interpretation formatted as
 a big-endian hex string, literal `-1' to indicate all processes or
 threads (respectively), or `0' to indicate an arbitrary process or
 thread. Specifying just a process, as `ppid', is equivalent to
 `ppid.-1'. It is an error to specify all processes but a specific
 thread, such as `p-1.tid'. Note that the `p' prefix is not used
 for those packets and replies explicitly documented to include a
 process ID, rather than a thread-id. "

... and we nowhere hardcoded 64-bit hex numbers.

Perhaps the previous patch was too invasive for such a simple need.
Since this is only required in the thread alive packet, and since
we already have to special case... would you be happy with the
attached patch?  I know I would.  :-)

-- 
Pedro Alves

[-- Attachment #2: thread_alive.diff --]
[-- Type: text/x-diff, Size: 1810 bytes --]

2008-10-23  Michael Snyder  <msnyder@vmware.com>
	    Pedro Alves  <pedro@codesourcery.com>

	* remote.c (remote_thread_alive): Emit leading zeros in the
	single-process case to preserve remote protocol behavior.

---
 gdb/remote.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-10-23 15:57:07.000000000 +0100
+++ src/gdb/remote.c	2008-10-23 16:30:19.000000000 +0100
@@ -1279,8 +1279,6 @@ static int
 remote_thread_alive (ptid_t ptid)
 {
   struct remote_state *rs = get_remote_state ();
-  int tid = ptid_get_tid (ptid);
-  char *p, *endp;
 
   if (ptid_equal (ptid, magic_null_ptid))
     /* The main thread is always alive.  */
@@ -1292,11 +1290,29 @@ remote_thread_alive (ptid_t ptid)
        multi-threading.  */
     return 1;
 
-  p = rs->buf;
-  endp = rs->buf + get_remote_packet_size ();
+  /* Note: We don't use write_ptid unconditionally here, since it
+     doesn't output leading zeros (%08x below).  Although nothing in
+     the docs suggests that the leading zeros are required, there's no
+     reason to break backwards compatibility, in case stubs are
+     relying on them being present.  */
+  if (!remote_multi_process_p (rs))
+    {
+      int tid = ptid_get_tid (ptid);
+
+      if (tid < 0)
+	xsnprintf (rs->buf, get_remote_packet_size (), "T-%08x", -tid);
+      else
+	xsnprintf (rs->buf, get_remote_packet_size (), "T%08x", tid);
+    }
+  else
+    {
+      char *p, *endp;
 
-  *p++ = 'T';
-  write_ptid (p, endp, ptid);
+      p = rs->buf;
+      endp = rs->buf + get_remote_packet_size ();
+      *p++ = 'T';
+      write_ptid (p, endp, ptid);
+    }
 
   putpkt (rs->buf);
   getpkt (&rs->buf, &rs->buf_size, 0);