* [PATCH] Clean up ptid.h/ptid.c.
@ 2013-10-03 14:09 Pedro Alves
2013-10-03 17:12 ` Tom Tromey
2013-10-04 7:30 ` Joel Brobecker
0 siblings, 2 replies; 4+ messages in thread
From: Pedro Alves @ 2013-10-03 14:09 UTC (permalink / raw)
To: gdb-patches
These files are quite small, but still managed to accumulate cruft. :-P
The ptid_t contructors, accessors and predicates are documented in
_three_ places, and each place uses a different wording.
E.g, the descriptions in the .c file of the new ptid_lwp_p, ptid_tid_p
weren't updated in the final revision like the descriptions in the .h
file were. Clearly, switching to a style that has a single central
description avoids such issues.
Worse, some of the existing descriptions are plain wrong, such as:
/* Attempt to find and return an existing ptid with the given PID, LWP,
and TID components. If none exists, create a new one and return
that. */
ptid_t ptid_build (int pid, long lwp, long tid);
The function does nothing that complicated. It's just a simple
constructor.
So this gets rid of all the unnecessary descriptions, leaving only the
ones near the function declarations in the header file, and
fixes/clarifies those that remain.
Comments?
gdb/
2013-10-04 Pedro Alves <palves@redhat.com>
* common/ptid.c (null_ptid, minus_one_ptid, ptid_build)
(pid_to_ptid, ptid_get_pid, ptid_get_lwp, ptid_get_tid)
(ptid_equal, ptid_is_pid, ptid_lwp_p, ptid_tid_p): Remove
describing comments.
* common/ptid.h: Remove intro description of constructors,
accessors and predicates.
(struct ptid): Reformat.
(minus_one_ptid, ptid_build, pid_to_ptid, ptid_get_pid)
(ptid_get_lwp, ptid_get_tid, ptid_equal, ptid_is_pid): Change
describing comments.
---
gdb/common/ptid.c | 19 ----------------
gdb/common/ptid.h | 65 ++++++++++++++++++++++---------------------------------
2 files changed, 26 insertions(+), 58 deletions(-)
diff --git a/gdb/common/ptid.c b/gdb/common/ptid.c
index e8d25c0..105a7ef 100644
--- a/gdb/common/ptid.c
+++ b/gdb/common/ptid.c
@@ -19,12 +19,9 @@
#include "ptid.h"
-/* Oft used ptids */
ptid_t null_ptid = { 0, 0, 0 };
ptid_t minus_one_ptid = { -1, 0, 0 };
-/* Create a ptid given the necessary PID, LWP, and TID components. */
-
ptid_t
ptid_build (int pid, long lwp, long tid)
{
@@ -36,40 +33,30 @@ ptid_build (int pid, long lwp, long tid)
return ptid;
}
-/* Create a ptid from just a pid. */
-
ptid_t
pid_to_ptid (int pid)
{
return ptid_build (pid, 0, 0);
}
-/* Fetch the pid (process id) component from a ptid. */
-
int
ptid_get_pid (ptid_t ptid)
{
return ptid.pid;
}
-/* Fetch the lwp (lightweight process) component from a ptid. */
-
long
ptid_get_lwp (ptid_t ptid)
{
return ptid.lwp;
}
-/* Fetch the tid (thread id) component from a ptid. */
-
long
ptid_get_tid (ptid_t ptid)
{
return ptid.tid;
}
-/* ptid_equal() is used to test equality of two ptids. */
-
int
ptid_equal (ptid_t ptid1, ptid_t ptid2)
{
@@ -78,8 +65,6 @@ ptid_equal (ptid_t ptid1, ptid_t ptid2)
&& ptid1.tid == ptid2.tid);
}
-/* Returns true if PTID represents a process. */
-
int
ptid_is_pid (ptid_t ptid)
{
@@ -90,8 +75,6 @@ ptid_is_pid (ptid_t ptid)
return (ptid_get_lwp (ptid) == 0 && ptid_get_tid (ptid) == 0);
}
-/* Returns true if PTID represents a lwp. */
-
int
ptid_lwp_p (ptid_t ptid)
{
@@ -102,8 +85,6 @@ ptid_lwp_p (ptid_t ptid)
return (ptid_get_lwp (ptid) != 0);
}
-/* Returns true if PTID represents a tid. */
-
int
ptid_tid_p (ptid_t ptid)
{
diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h
index 1326506..2a31a1c 100644
--- a/gdb/common/ptid.h
+++ b/gdb/common/ptid.h
@@ -20,69 +20,56 @@
#ifndef PTID_H
#define PTID_H
-/* The ptid struct is a collection of the various "ids" necessary
- for identifying the inferior. This consists of the process id
- (pid), thread id (tid), and other fields necessary for uniquely
- identifying the inferior process/thread being debugged. When
- manipulating ptids, the constructors, accessors, and predicates
- declared in this file should be used. These are as follows:
-
- ptid_build - Make a new ptid from a pid, lwp, and tid.
- pid_to_ptid - Make a new ptid from just a pid.
- ptid_get_pid - Fetch the pid component of a ptid.
- ptid_get_lwp - Fetch the lwp component of a ptid.
- ptid_get_tid - Fetch the tid component of a ptid.
- ptid_equal - Test to see if two ptids are equal.
- ptid_is_pid - Test if a ptid is of the form (pid, 0, 0).
- ptid_lwp_p - Test if a ptid's lwp component is non-zero.
- ptid_tid_p - Test if a ptid's tid component is non-zero.
-
- Please do NOT access the struct ptid members directly (except, of
- course, in the implementation of the above ptid manipulation
- functions). */
+/* 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
+ thread id (tid). When manipulating ptids, the constructors,
+ accessors, and predicates declared in this file should be used. Do
+ NOT access the struct ptid members directly. */
struct ptid
- {
- /* Process id */
- int pid;
+{
+ /* Process id. */
+ int pid;
- /* Lightweight process id */
- long lwp;
+ /* Lightweight process id. */
+ long lwp;
- /* Thread id */
- long tid;
- };
+ /* Thread id. */
+ long tid;
+};
typedef struct ptid ptid_t;
/* The null or zero ptid, often used to indicate no process. */
extern ptid_t null_ptid;
-/* The -1 ptid, often used to indicate either an error condition
+/* 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;
-/* Attempt to find and return an existing ptid with the given PID, LWP,
- and TID components. If none exists, create a new one and return
- that. */
+/* Make a ptid given the necessary PID, LWP, and TID components. */
ptid_t ptid_build (int pid, long lwp, long tid);
-/* Find/Create a ptid from just a pid. */
+/* Make a new ptid from just a pid. This ptid is usually used to
+ represent a whole process, including all its lwps/threads. */
ptid_t pid_to_ptid (int pid);
-/* Fetch the pid (process id) component from a ptid. */
+/* Fetch the pid (process id) component from a ptid. */
int ptid_get_pid (ptid_t ptid);
-/* Fetch the lwp (lightweight process) component from a ptid. */
+/* Fetch the lwp (lightweight process) component from a ptid. */
long ptid_get_lwp (ptid_t ptid);
-/* Fetch the tid (thread id) component from a ptid. */
+/* Fetch the tid (thread id) component from a ptid. */
long ptid_get_tid (ptid_t ptid);
-/* Compare two ptids to see if they are equal */
-int ptid_equal (ptid_t p1, ptid_t p2);
+/* Compare two ptids to see if they are equal. */
+int ptid_equal (ptid_t ptid1, ptid_t ptid2);
-/* Return true if PTID represents a process id. */
+/* 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);
/* Return true if PTID's lwp member is non-zero. */
--
1.7.11.7
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Clean up ptid.h/ptid.c. 2013-10-03 14:09 [PATCH] Clean up ptid.h/ptid.c Pedro Alves @ 2013-10-03 17:12 ` Tom Tromey 2013-10-04 10:07 ` Pedro Alves 2013-10-04 7:30 ` Joel Brobecker 1 sibling, 1 reply; 4+ messages in thread From: Tom Tromey @ 2013-10-03 17:12 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> So this gets rid of all the unnecessary descriptions, leaving only the Pedro> ones near the function declarations in the header file, and Pedro> fixes/clarifies those that remain. Pedro> Comments? Looks good to me. It's my preferred approach. Pedro> -/* Create a ptid given the necessary PID, LWP, and TID components. */ Pedro> - Pedro> ptid_t Pedro> ptid_build (int pid, long lwp, long tid) Pedro> { I usually write /* See ptid.h. */ in these spots. Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Clean up ptid.h/ptid.c. 2013-10-03 17:12 ` Tom Tromey @ 2013-10-04 10:07 ` Pedro Alves 0 siblings, 0 replies; 4+ messages in thread From: Pedro Alves @ 2013-10-04 10:07 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Joel Brobecker On 10/04/2013 08:30 AM, Joel Brobecker wrote: > I like it! Thanks for doing this cleanup, Pedro. Great! You're welcome. On 10/03/2013 06:11 PM, Tom Tromey wrote: > Looks good to me. > It's my preferred approach. > I usually write /* See ptid.h. */ in these spots. Thanks, added and checked in, as below. ----- Clean up ptid.h/ptid.c. The ptid_t contructors, accessors and predicates are documented in _three_ places, and each place uses a different wording. E.g, the descriptions in the .c file of the new ptid_lwp_p, ptid_tid_p weren't updated in the final revision like the descriptions in the .h file were. Clearly, switching to a style that has a single central description avoids such issues. Worse, some of the existing descriptions are plain wrong, such as: /* Attempt to find and return an existing ptid with the given PID, LWP, and TID components. If none exists, create a new one and return that. */ ptid_t ptid_build (int pid, long lwp, long tid); The function does nothing that complicated. It's just a simple constructor. So this gets rid of all the unnecessary descriptions, leaving only the ones near the function declarations in the header file, and fixes/clarifies those that remain. gdb/ 2013-10-04 Pedro Alves <palves@redhat.com> * common/ptid.c (null_ptid, minus_one_ptid, ptid_build) (pid_to_ptid, ptid_get_pid, ptid_get_lwp, ptid_get_tid) (ptid_equal, ptid_is_pid, ptid_lwp_p, ptid_tid_p): Replace describing comments with references to ptid.h. * common/ptid.h: Remove intro description of constructors, accessors and predicates. (struct ptid): Reformat. (minus_one_ptid, ptid_build, pid_to_ptid, ptid_get_pid) (ptid_get_lwp, ptid_get_tid, ptid_equal, ptid_is_pid): Change describing comments. --- gdb/common/ptid.c | 21 +++++++++-------- gdb/common/ptid.h | 65 +++++++++++++++++++++-------------------------------- 2 files changed, 37 insertions(+), 49 deletions(-) diff --git a/gdb/common/ptid.c b/gdb/common/ptid.c index e8d25c0..d7780d3 100644 --- a/gdb/common/ptid.c +++ b/gdb/common/ptid.c @@ -19,11 +19,12 @@ #include "ptid.h" -/* Oft used ptids */ +/* See ptid.h for these. */ + ptid_t null_ptid = { 0, 0, 0 }; ptid_t minus_one_ptid = { -1, 0, 0 }; -/* Create a ptid given the necessary PID, LWP, and TID components. */ +/* See ptid.h. */ ptid_t ptid_build (int pid, long lwp, long tid) @@ -36,7 +37,7 @@ ptid_build (int pid, long lwp, long tid) return ptid; } -/* Create a ptid from just a pid. */ +/* See ptid.h. */ ptid_t pid_to_ptid (int pid) @@ -44,7 +45,7 @@ pid_to_ptid (int pid) return ptid_build (pid, 0, 0); } -/* Fetch the pid (process id) component from a ptid. */ +/* See ptid.h. */ int ptid_get_pid (ptid_t ptid) @@ -52,7 +53,7 @@ ptid_get_pid (ptid_t ptid) return ptid.pid; } -/* Fetch the lwp (lightweight process) component from a ptid. */ +/* See ptid.h. */ long ptid_get_lwp (ptid_t ptid) @@ -60,7 +61,7 @@ ptid_get_lwp (ptid_t ptid) return ptid.lwp; } -/* Fetch the tid (thread id) component from a ptid. */ +/* See ptid.h. */ long ptid_get_tid (ptid_t ptid) @@ -68,7 +69,7 @@ ptid_get_tid (ptid_t ptid) return ptid.tid; } -/* ptid_equal() is used to test equality of two ptids. */ +/* See ptid.h. */ int ptid_equal (ptid_t ptid1, ptid_t ptid2) @@ -78,7 +79,7 @@ ptid_equal (ptid_t ptid1, ptid_t ptid2) && ptid1.tid == ptid2.tid); } -/* Returns true if PTID represents a process. */ +/* See ptid.h. */ int ptid_is_pid (ptid_t ptid) @@ -90,7 +91,7 @@ ptid_is_pid (ptid_t ptid) return (ptid_get_lwp (ptid) == 0 && ptid_get_tid (ptid) == 0); } -/* Returns true if PTID represents a lwp. */ +/* See ptid.h. */ int ptid_lwp_p (ptid_t ptid) @@ -102,7 +103,7 @@ ptid_lwp_p (ptid_t ptid) return (ptid_get_lwp (ptid) != 0); } -/* Returns true if PTID represents a tid. */ +/* See ptid.h. */ int ptid_tid_p (ptid_t ptid) diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h index 1326506..2a31a1c 100644 --- a/gdb/common/ptid.h +++ b/gdb/common/ptid.h @@ -20,69 +20,56 @@ #ifndef PTID_H #define PTID_H -/* The ptid struct is a collection of the various "ids" necessary - for identifying the inferior. This consists of the process id - (pid), thread id (tid), and other fields necessary for uniquely - identifying the inferior process/thread being debugged. When - manipulating ptids, the constructors, accessors, and predicates - declared in this file should be used. These are as follows: - - ptid_build - Make a new ptid from a pid, lwp, and tid. - pid_to_ptid - Make a new ptid from just a pid. - ptid_get_pid - Fetch the pid component of a ptid. - ptid_get_lwp - Fetch the lwp component of a ptid. - ptid_get_tid - Fetch the tid component of a ptid. - ptid_equal - Test to see if two ptids are equal. - ptid_is_pid - Test if a ptid is of the form (pid, 0, 0). - ptid_lwp_p - Test if a ptid's lwp component is non-zero. - ptid_tid_p - Test if a ptid's tid component is non-zero. - - Please do NOT access the struct ptid members directly (except, of - course, in the implementation of the above ptid manipulation - functions). */ +/* 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 + thread id (tid). When manipulating ptids, the constructors, + accessors, and predicates declared in this file should be used. Do + NOT access the struct ptid members directly. */ struct ptid - { - /* Process id */ - int pid; +{ + /* Process id. */ + int pid; - /* Lightweight process id */ - long lwp; + /* Lightweight process id. */ + long lwp; - /* Thread id */ - long tid; - }; + /* Thread id. */ + long tid; +}; typedef struct ptid ptid_t; /* The null or zero ptid, often used to indicate no process. */ extern ptid_t null_ptid; -/* The -1 ptid, often used to indicate either an error condition +/* 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; -/* Attempt to find and return an existing ptid with the given PID, LWP, - and TID components. If none exists, create a new one and return - that. */ +/* Make a ptid given the necessary PID, LWP, and TID components. */ ptid_t ptid_build (int pid, long lwp, long tid); -/* Find/Create a ptid from just a pid. */ +/* Make a new ptid from just a pid. This ptid is usually used to + represent a whole process, including all its lwps/threads. */ ptid_t pid_to_ptid (int pid); -/* Fetch the pid (process id) component from a ptid. */ +/* Fetch the pid (process id) component from a ptid. */ int ptid_get_pid (ptid_t ptid); -/* Fetch the lwp (lightweight process) component from a ptid. */ +/* Fetch the lwp (lightweight process) component from a ptid. */ long ptid_get_lwp (ptid_t ptid); -/* Fetch the tid (thread id) component from a ptid. */ +/* Fetch the tid (thread id) component from a ptid. */ long ptid_get_tid (ptid_t ptid); -/* Compare two ptids to see if they are equal */ -int ptid_equal (ptid_t p1, ptid_t p2); +/* Compare two ptids to see if they are equal. */ +int ptid_equal (ptid_t ptid1, ptid_t ptid2); -/* Return true if PTID represents a process id. */ +/* 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); /* Return true if PTID's lwp member is non-zero. */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Clean up ptid.h/ptid.c. 2013-10-03 14:09 [PATCH] Clean up ptid.h/ptid.c Pedro Alves 2013-10-03 17:12 ` Tom Tromey @ 2013-10-04 7:30 ` Joel Brobecker 1 sibling, 0 replies; 4+ messages in thread From: Joel Brobecker @ 2013-10-04 7:30 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches > So this gets rid of all the unnecessary descriptions, leaving only the > ones near the function declarations in the header file, and > fixes/clarifies those that remain. > > Comments? I like it! Thanks for doing this cleanup, Pedro. > gdb/ > 2013-10-04 Pedro Alves <palves@redhat.com> > > * common/ptid.c (null_ptid, minus_one_ptid, ptid_build) > (pid_to_ptid, ptid_get_pid, ptid_get_lwp, ptid_get_tid) > (ptid_equal, ptid_is_pid, ptid_lwp_p, ptid_tid_p): Remove > describing comments. > * common/ptid.h: Remove intro description of constructors, > accessors and predicates. > (struct ptid): Reformat. > (minus_one_ptid, ptid_build, pid_to_ptid, ptid_get_pid) > (ptid_get_lwp, ptid_get_tid, ptid_equal, ptid_is_pid): Change > describing comments. -- Joel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-04 10:07 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-10-03 14:09 [PATCH] Clean up ptid.h/ptid.c Pedro Alves 2013-10-03 17:12 ` Tom Tromey 2013-10-04 10:07 ` Pedro Alves 2013-10-04 7:30 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox