From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1667 invoked by alias); 23 Oct 2008 15:39:14 -0000 Received: (qmail 1611 invoked by uid 22791); 23 Oct 2008 15:39:13 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 23 Oct 2008 15:38:38 +0000 Received: (qmail 23292 invoked from network); 23 Oct 2008 15:38:36 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 23 Oct 2008 15:38:36 -0000 From: Pedro Alves To: Michael Snyder Subject: Re: [RFA] Restore leading zeros in remote_thread_alive Date: Thu, 23 Oct 2008 15:39:00 -0000 User-Agent: KMail/1.9.9 Cc: "gdb-patches@sourceware.org" References: <48FE7B9B.3040905@vmware.com> <200810221516.09164.pedro@codesourcery.com> <48FF6B55.4030902@vmware.com> In-Reply-To: <48FF6B55.4030902@vmware.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_6pJAJbXevBRdLWd" Message-Id: <200810231638.34327.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00576.txt.bz2 --Boundary-00=_6pJAJbXevBRdLWd Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1941 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 --Boundary-00=_6pJAJbXevBRdLWd Content-Type: text/x-diff; charset="utf-8"; name="thread_alive.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="thread_alive.diff" Content-length: 1810 2008-10-23 Michael Snyder Pedro Alves * 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); --Boundary-00=_6pJAJbXevBRdLWd--