This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Comment on '(ptid_get_lwp (ptid) == -1'
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 12 Jun 2012 13:13:32 +0100
- Subject: Re: [PATCH] Comment on '(ptid_get_lwp (ptid) == -1'
- References: <1339498212-2247-1-git-send-email-yao@codesourcery.com>
On 06/12/2012 11:50 AM, Yao Qi wrote:
> if (ptid_equal (ptid, minus_one_ptid)
> || ptid_equal (ptid, entry->id)
> || (ptid_is_pid (ptid)
> && (ptid_get_pid (ptid) == pid_of (lwp)))
> + /* The "thread-id" syntax in RSP specifies a process as "'pPID.-1'.
^^ ^
(Quoting looks odd.)
Not exactly the right way to spell it. pPID.-1 stands for "all threads of
process PID". And then the thread-id description says that "Specifying just
a process, as @samp{p@var{pid}}, is equivalent to @samp{p@var{pid}.-1}".
In practice the same, but it's defined the other way around.
> + When ptid_get_lwp (ptid) returns -1, it means PTID stands for a
> + process. */
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 9fd550b..30451a7 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1978,6 +1978,9 @@ handle_v_cont (char *own_buf)
> vCont. */
> if (n == 1
> && !(ptid_equal (resume_info[0].thread, minus_one_ptid)
> + /* The "thread-id" syntax in RSP specifies a process as "'pPID.-1'.
> + When ptid_get_lwp (ptid) returns -1, it means PTID stands for a
> + process. */
> || ptid_get_lwp (resume_info[0].thread) == -1)
> && resume_info[0].kind != resume_stop)
> cont_thread = resume_info[0].thread;
I find the large embedded comment a bit more distracting than desirable. We already
say above that we're handling wildcard vConts. Would the change below make things
clear enough in your view? I'd rather leave further clarifications of the basics
of the protocol to the manual.
(I think linux_set_resume_request handling 'pPID'/'pPID.0' might have been
defensive code; I don't recall a time when GDB sent that in a vCont.)
2012-06-12 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* linux-low.c (linux_set_resume_request): Simplify predicate. Add
comment.
* server.c (handle_v_cont): Extend comment.
---
gdb/gdbserver/linux-low.c | 9 +++++----
gdb/gdbserver/server.c | 4 ++--
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 3e88c42..3d116fd 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -3339,10 +3339,11 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
ptid_t ptid = r->resume[ndx].thread;
if (ptid_equal (ptid, minus_one_ptid)
|| ptid_equal (ptid, entry->id)
- || (ptid_is_pid (ptid)
- && (ptid_get_pid (ptid) == pid_of (lwp)))
- || (ptid_get_lwp (ptid) == -1
- && (ptid_get_pid (ptid) == pid_of (lwp))))
+ /* Handle both 'pPID' and 'pPID.-1' as meaning 'all threads
+ of PID'. */
+ || (ptid_get_pid (ptid) == pid_of (lwp)
+ && (ptid_is_pid (ptid)
+ || ptid_get_lwp (ptid) == -1)))
{
if (r->resume[ndx].kind == resume_stop
&& thread->last_resume_kind == resume_stop)
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 9fd550b..96aa54b 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1974,8 +1974,8 @@ handle_v_cont (char *own_buf)
/* `cont_thread' is still used in occasional places in the backend,
to implement single-thread scheduler-locking. Doesn't make sense
- to set it if we see a stop request, or any form of wildcard
- vCont. */
+ to set it if we see a stop request, or a wildcard action (one
+ with '-1' (all threads), or 'pPID.-1' (all threads of PID)). */
if (n == 1
&& !(ptid_equal (resume_info[0].thread, minus_one_ptid)
|| ptid_get_lwp (resume_info[0].thread) == -1)