diff mbox

[v2] Use "switch_to_thread" more thoroughly on gdbserver

Message ID 20170926042239.30748-1-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Sept. 26, 2017, 4:22 a.m. UTC 
  This is a technical debt that I left when I ported "switch_to_thread"
to gdbserver.  It's a simple patch that converts occurences of:

  current_thread = find_thread_ptid (ptid);

to:

  switch_to_thread (ptid);

This patch also converts a simple "if" to a "gdb_assert" on
gdbserver's version of "switch_to_thread":

  gdb_assert (!ptid_equal (ptid, minus_one_ptid));

This change makes the code more similar to what GDB already does.

Regtested on BuildBot.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdbthread.h: Include "common-gdbthread.h".
	* inferiors.c (switch_to_thread): Use "gdb_assert" instead of
	"if" when validating the ptid.
	* remote-utils.c: Include "gdbthread.h".
	(prepare_resume_reply): Use "switch_to_thread".
	* target.c (done_accessing_memory): Likewise.
---
 gdb/gdbserver/gdbthread.h    | 1 +
 gdb/gdbserver/inferiors.c    | 4 ++--
 gdb/gdbserver/remote-utils.c | 3 ++-
 gdb/gdbserver/target.c       | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

Comments

Pedro Alves Sept. 26, 2017, 12:28 p.m. UTC | #1 
On 09/26/2017 05:22 AM, Sergio Durigan Junior wrote:

> gdb/gdbserver/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* gdbthread.h: Include "common-gdbthread.h".
> 	* inferiors.c (switch_to_thread): Use "gdb_assert" instead of
> 	"if" when validating the ptid.
> 	* remote-utils.c: Include "gdbthread.h".
> 	(prepare_resume_reply): Use "switch_to_thread".
> 	* target.c (done_accessing_memory): Likewise.

OK

>  void
>  switch_to_thread (ptid_t ptid)
>  {
> -  if (!ptid_equal (ptid, minus_one_ptid))
> -    current_thread = find_thread_ptid (ptid);
> +  gdb_assert (!ptid_equal (ptid, minus_one_ptid));

Note you can write:

  gdb_assert (ptid != minus_one_ptid);

nowadays.

Thanks,
Pedro Alves
Sergio Durigan Junior Sept. 26, 2017, 4:52 p.m. UTC | #2 
On Tuesday, September 26 2017, Pedro Alves wrote:

> On 09/26/2017 05:22 AM, Sergio Durigan Junior wrote:
>
>> gdb/gdbserver/ChangeLog:
>> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
>> 
>> 	* gdbthread.h: Include "common-gdbthread.h".
>> 	* inferiors.c (switch_to_thread): Use "gdb_assert" instead of
>> 	"if" when validating the ptid.
>> 	* remote-utils.c: Include "gdbthread.h".
>> 	(prepare_resume_reply): Use "switch_to_thread".
>> 	* target.c (done_accessing_memory): Likewise.
>
> OK
>
>>  void
>>  switch_to_thread (ptid_t ptid)
>>  {
>> -  if (!ptid_equal (ptid, minus_one_ptid))
>> -    current_thread = find_thread_ptid (ptid);
>> +  gdb_assert (!ptid_equal (ptid, minus_one_ptid));
>
> Note you can write:
>
>   gdb_assert (ptid != minus_one_ptid);
>
> nowadays.

That's right; pushed with this small nit fixed.

75352e283fb2b265d14c750859156943f6eb2693

Thanks,
diff mbox

Patch

diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index 04cc47ba71..a864f95884 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -19,6 +19,7 @@ 
 #ifndef GDB_THREAD_H
 #define GDB_THREAD_H
 
+#include "common-gdbthread.h"
 #include "inferiors.h"
 
 struct btrace_target_info;
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 72f0412757..7705c2af47 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -442,6 +442,6 @@  make_cleanup_restore_current_thread (void)
 void
 switch_to_thread (ptid_t ptid)
 {
-  if (!ptid_equal (ptid, minus_one_ptid))
-    current_thread = find_thread_ptid (ptid);
+  gdb_assert (!ptid_equal (ptid, minus_one_ptid));
+  current_thread = find_thread_ptid (ptid);
 }
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 25b7e41a91..761604683c 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -23,6 +23,7 @@ 
 #include "tdesc.h"
 #include "dll.h"
 #include "rsp-low.h"
+#include "gdbthread.h"
 #include <ctype.h>
 #if HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
@@ -1188,7 +1189,7 @@  prepare_resume_reply (char *buf, ptid_t ptid,
 
 	saved_thread = current_thread;
 
-	current_thread = find_thread_ptid (ptid);
+	switch_to_thread (ptid);
 
 	regp = current_target_desc ()->expedite_regs;
 
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 7f5bed7ec7..94224a8a7c 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -138,7 +138,7 @@  done_accessing_memory (void)
 
   /* Restore the previous selected thread.  */
   general_thread = prev_general_thread;
-  current_thread = find_thread_ptid (general_thread);
+  switch_to_thread (general_thread);
 }
 
 int