Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@ericsson.com>
To: <gdb-patches@sourceware.org>
Cc: Simon Marchi <simon.marchi@ericsson.com>
Subject: [PATCH 2/2] Class-ify ptid_t
Date: Tue, 04 Apr 2017 18:33:00 -0000	[thread overview]
Message-ID: <20170404183235.10589-2-simon.marchi@ericsson.com> (raw)
In-Reply-To: <20170404183235.10589-1-simon.marchi@ericsson.com>

I grew a bit tired of using ptid_get_{lwp,pid,tid} and friends, so I
decided to make it a bit easier to use by making it a proper class.

Because ptid_t is used in things that aren't constructed, it is not
possible to have a constructor.  Instead I added a "build" static
method, which maps well to the current ptid_build anyway, and ptid_t is
basically just a plain old data type with read-only methods.  The
difference with before is that the fields are private, so it's not
possible to change a ptid_t field by mistake.

The new methods of ptid_t map to existing functions/practice like this:

  ptid_t::build (pid, lwp, tid) -> ptid_build (pid, lwp, tid)
  ptid_t::build (pid) -> pid_to_ptid (pid)
  ptid.is_pid () -> ptid_is_pid (ptid)
  ptid == other -> ptid_equal (ptid, other)
  ptid.is_null () -> ptid_equal (ptid, null_ptid)
  ptid.is_any () -> ptid_equal (ptid, minus_one_ptid)
  ptid.pid () -> ptid_get_pid (ptid)
  ptid.lwp_p () -> ptid_lwp_p (ptid)
  ptid.lwp () -> ptid_get_lwp (ptid)
  ptid.tid_p () -> ptid_tid_p (ptid)
  ptid.tid () -> ptid_get_tid (ptid)
  ptid.matches (filter) -> ptid_match (ptid, filter)

I've replaced the implementation of the existing functions with calls to
the new methods.  People are encouraged to gradually switch to using the
ptid_t methods instead of the functions (or we can change them all in
one pass eventually).

Also, I'm not sure if it's worth it (because of ptid_t's relatively
small size), but I have made the functions and methods take ptid_t
arguments by const reference instead of by value.

gdb/ChangeLog:

	* common/ptid.h (struct ptid): Change to...
	(class ptid_t): ... this.
	<build>: New static method.
	<is_pid, is_null, is_any, pid, lwp_p, lwp, tid_p, tid,
	operator==, matches>: New methods.
	<pid>: Rename to...
	<m_pid>: ...this.
	<lwp>: Rename to...
	<m_lwp>: ...this.
	<tid>: Rename to...
	<m_tid>: ...this.
	(ptid_get_pid, ptid_get_lwp, ptid_get_tid, ptid_equal,
	ptid_is_pid, ptid_lwp_p, ptid_tid_p, ptid_match): Take arguments
	as references.
	* common/ptid.c (null_ptid, minus_one_ptid): Initialize with
	ptid_t::build.
	(ptid_build, pid_to_ptid, ptid_get_pid, ptid_get_tid,
	ptid_equal, ptid_is_pid, ptid_lwp_p, ptid_tid_p, ptid_match):
	Take arguments as references, implement using ptid_t methods.

gdb/gdbserver/ChangeLog:

	* server.c (handle_v_cont): Initialize thread_resume::thread
	with null_ptid.
---
 gdb/common/ptid.c      |  62 +++++++++++-----------------
 gdb/common/ptid.h      | 109 +++++++++++++++++++++++++++++++++++++++----------
 gdb/gdbserver/server.c |   4 +-
 3 files changed, 114 insertions(+), 61 deletions(-)

diff --git a/gdb/common/ptid.c b/gdb/common/ptid.c
index b56971b9af..ca51a4ec7d 100644
--- a/gdb/common/ptid.c
+++ b/gdb/common/ptid.c
@@ -22,20 +22,16 @@
 
 /* See ptid.h for these.  */
 
-ptid_t null_ptid = { 0, 0, 0 };
-ptid_t minus_one_ptid = { -1, 0, 0 };
+ptid_t null_ptid = ptid_t::build (0, 0, 0);
+ptid_t minus_one_ptid = ptid_t::build (-1, 0, 0);
+
 
 /* See ptid.h.  */
 
 ptid_t
 ptid_build (int pid, long lwp, long tid)
 {
-  ptid_t ptid;
-
-  ptid.pid = pid;
-  ptid.lwp = lwp;
-  ptid.tid = tid;
-  return ptid;
+  return ptid_t::build (pid, lwp, tid);
 }
 
 /* See ptid.h.  */
@@ -43,81 +39,69 @@ ptid_build (int pid, long lwp, long tid)
 ptid_t
 pid_to_ptid (int pid)
 {
-  return ptid_build (pid, 0, 0);
+  return ptid_t::build (pid);
 }
 
 /* See ptid.h.  */
 
 int
-ptid_get_pid (ptid_t ptid)
+ptid_get_pid (const ptid_t &ptid)
 {
-  return ptid.pid;
+  return ptid.pid ();
 }
 
 /* See ptid.h.  */
 
 long
-ptid_get_lwp (ptid_t ptid)
+ptid_get_lwp (const ptid_t &ptid)
 {
-  return ptid.lwp;
+  return ptid.lwp ();
 }
 
 /* See ptid.h.  */
 
 long
-ptid_get_tid (ptid_t ptid)
+ptid_get_tid (const ptid_t &ptid)
 {
-  return ptid.tid;
+  return ptid.tid ();
 }
 
 /* See ptid.h.  */
 
 int
-ptid_equal (ptid_t ptid1, ptid_t ptid2)
+ptid_equal (const ptid_t &ptid1, const ptid_t &ptid2)
 {
-  return (ptid1.pid == ptid2.pid
-	  && ptid1.lwp == ptid2.lwp
-	  && ptid1.tid == ptid2.tid);
+  return ptid1 == ptid2;
 }
 
 /* See ptid.h.  */
 
 int
-ptid_is_pid (ptid_t ptid)
+ptid_is_pid (const ptid_t &ptid)
 {
-  if (ptid_equal (minus_one_ptid, ptid)
-      || ptid_equal (null_ptid, ptid))
-    return 0;
-
-  return (ptid_get_lwp (ptid) == 0 && ptid_get_tid (ptid) == 0);
+  return ptid.is_pid ();
 }
 
 /* See ptid.h.  */
 
 int
-ptid_lwp_p (ptid_t ptid)
+ptid_lwp_p (const ptid_t &ptid)
 {
-  return (ptid_get_lwp (ptid) != 0);
+  return ptid.lwp_p ();
 }
 
 /* See ptid.h.  */
 
 int
-ptid_tid_p (ptid_t ptid)
+ptid_tid_p (const ptid_t &ptid)
 {
-  return (ptid_get_tid (ptid) != 0);
+  return ptid.tid_p ();
 }
 
+/* See ptid.h.  */
+
 int
-ptid_match (ptid_t ptid, ptid_t filter)
+ptid_match (const ptid_t &ptid, const ptid_t &filter)
 {
-  if (ptid_equal (filter, minus_one_ptid))
-    return 1;
-  if (ptid_is_pid (filter)
-      && ptid_get_pid (ptid) == ptid_get_pid (filter))
-    return 1;
-  else if (ptid_equal (ptid, filter))
-    return 1;
-
-  return 0;
+  return ptid.matches (filter);
 }
diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h
index 337bfb0899..c8649ae9a8 100644
--- a/gdb/common/ptid.h
+++ b/gdb/common/ptid.h
@@ -20,6 +20,15 @@
 #ifndef PTID_H
 #define PTID_H
 
+class ptid_t;
+
+/* The null or zero ptid, often used to indicate no process.  */
+extern ptid_t null_ptid;
+
+/* The (-1,0,0) ptid, often used to indicate either an error condition
+   or a "don't care" condition, i.e, "run all threads."  */
+extern ptid_t minus_one_ptid;
+
 /* The ptid struct is a collection of the various "ids" necessary for
    identifying the inferior process/thread being debugged.  This
    consists of the process id (pid), lightweight process id (lwp) and
@@ -32,27 +41,85 @@
    thread_stratum target that might want to sit on top.
 */
 
-struct ptid
+class ptid_t
 {
+public:
+  static ptid_t build (int pid, long lwp = 0, long tid = 0)
+  {
+    ptid_t ptid;
+
+    ptid.m_pid = pid;
+    ptid.m_lwp = lwp;
+    ptid.m_tid = tid;
+
+    return ptid;
+  }
+
+  bool is_pid () const
+  {
+    if (is_any () || is_null())
+      return false;
+
+    return m_lwp == 0 && m_tid == 0;
+  }
+
+  bool is_null () const
+  {
+    return *this == null_ptid;
+  }
+
+  bool is_any () const
+  {
+    return *this == minus_one_ptid;
+  }
+
+  int pid () const
+  { return m_pid; }
+
+  bool lwp_p () const
+  { return m_lwp != 0; }
+
+  long lwp () const
+  { return m_lwp; }
+
+  bool tid_p () const
+  { return m_tid != 0; }
+
+  long tid () const
+  { return m_tid; }
+
+  bool operator== (const ptid_t &other) const
+  {
+    return (m_pid == other.m_pid
+	    && m_lwp == other.m_lwp
+	    && m_tid == other.m_tid);
+  }
+
+  bool matches (const ptid_t &filter) const
+  {
+    /* If filter represents any ptid, it's always a match.  */
+    if (filter.is_any ())
+      return true;
+
+    /* If filter is only a pid, any ptid with that pid matches.  */
+    if (filter.is_pid () && m_pid == filter.pid ())
+      return true;
+
+    /* Otherwise, this ptid only matches if it's exactly equal to filter.  */
+    return *this == filter;
+  }
+
+private:
   /* Process id.  */
-  int pid;
+  int m_pid;
 
   /* Lightweight process id.  */
-  long lwp;
+  long m_lwp;
 
   /* Thread id.  */
-  long tid;
+  long m_tid;
 };
 
-typedef struct ptid ptid_t;
-
-/* The null or zero ptid, often used to indicate no process. */
-extern ptid_t null_ptid;
-
-/* The (-1,0,0) ptid, often used to indicate either an error condition
-   or a "don't care" condition, i.e, "run all threads."  */
-extern ptid_t minus_one_ptid;
-
 /* Make a ptid given the necessary PID, LWP, and TID components.  */
 ptid_t ptid_build (int pid, long lwp, long tid);
 
@@ -61,27 +128,27 @@ ptid_t ptid_build (int pid, long lwp, long tid);
 ptid_t pid_to_ptid (int pid);
 
 /* Fetch the pid (process id) component from a ptid.  */
-int ptid_get_pid (ptid_t ptid);
+int ptid_get_pid (const ptid_t &ptid);
 
 /* Fetch the lwp (lightweight process) component from a ptid.  */
-long ptid_get_lwp (ptid_t ptid);
+long ptid_get_lwp (const ptid_t &ptid);
 
 /* Fetch the tid (thread id) component from a ptid.  */
-long ptid_get_tid (ptid_t ptid);
+long ptid_get_tid (const ptid_t &ptid);
 
 /* Compare two ptids to see if they are equal.  */
-int ptid_equal (ptid_t ptid1, ptid_t ptid2);
+int ptid_equal (const ptid_t &ptid1, const ptid_t &ptid2);
 
 /* Returns true if PTID represents a whole process, including all its
    lwps/threads.  Such ptids have the form of (pid,0,0), with pid !=
    -1.  */
-int ptid_is_pid (ptid_t ptid);
+int ptid_is_pid (const ptid_t &ptid);
 
 /* Return true if PTID's lwp member is non-zero.  */
-int ptid_lwp_p (ptid_t ptid);
+int ptid_lwp_p (const ptid_t &ptid);
 
 /* Return true if PTID's tid member is non-zero.  */
-int ptid_tid_p (ptid_t ptid);
+int ptid_tid_p (const ptid_t &ptid);
 
 /* Returns true if PTID matches filter FILTER.  FILTER can be the wild
    card MINUS_ONE_PTID (all ptid match it); can be a ptid representing
@@ -91,6 +158,6 @@ int ptid_tid_p (ptid_t ptid);
    case, only that thread will match true.  PTID must represent a
    specific LWP or THREAD, it can never be a wild card.  */
 
-extern int ptid_match (ptid_t ptid, ptid_t filter);
+extern int ptid_match (const ptid_t &ptid, const ptid_t &filter);
 
 #endif
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 4bc7f71b00..1287114cc1 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2654,7 +2654,9 @@ handle_v_cont (char *own_buf)
   char *p, *q;
   int n = 0, i = 0;
   struct thread_resume *resume_info;
-  struct thread_resume default_action = {{0}};
+  struct thread_resume default_action = {
+    .thread = null_ptid,
+  };
 
   /* Count the number of semicolons in the packet.  There should be one
      for every action.  */
-- 
2.11.0


  reply	other threads:[~2017-04-04 18:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 18:32 [PATCH 1/2] ptid_{lwp,tid}_p: Remove unnecessary checks Simon Marchi
2017-04-04 18:33 ` Simon Marchi [this message]
2017-04-05 15:47   ` [PATCH 2/2] Class-ify ptid_t Pedro Alves
2017-04-05 19:44     ` Simon Marchi
2017-04-05 21:31       ` Pedro Alves
2017-04-06  2:15         ` Simon Marchi
2017-04-06 10:49           ` Pedro Alves
2017-04-06 11:12             ` Pedro Alves
2017-04-06 14:32               ` Simon Marchi
2017-04-06 14:38                 ` Pedro Alves
2017-04-06  3:09         ` Simon Marchi
2017-04-06 11:06           ` Pedro Alves
2017-04-05 15:15 ` [PATCH 1/2] ptid_{lwp,tid}_p: Remove unnecessary checks Pedro Alves
2017-04-05 19:21   ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170404183235.10589-2-simon.marchi@ericsson.com \
    --to=simon.marchi@ericsson.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox