[PATCH 1/6] gdb/linux-nat: use get_ptrace_pid in two spots

[PATCH 1/6] gdb/linux-nat: use get_ptrace_pid in two spots

[Simon Marchi]

No behavior change expected.

Change-Id: Ifaa64ecd619483646b024fd7c62e571e92a8eedb
---
 gdb/linux-nat.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 17e5dce08c3d..9ab8b845cbb4 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3611,20 +3611,16 @@ linux_xfer_siginfo (enum target_object object,
 		    const gdb_byte *writebuf, ULONGEST offset, ULONGEST len,
 		    ULONGEST *xfered_len)
 {
-  int pid;
   siginfo_t siginfo;
   gdb_byte inf_siginfo[sizeof (siginfo_t)];
 
   gdb_assert (object == TARGET_OBJECT_SIGNAL_INFO);
   gdb_assert (readbuf || writebuf);
 
-  pid = inferior_ptid.lwp ();
-  if (pid == 0)
-    pid = inferior_ptid.pid ();
-
   if (offset > sizeof (siginfo))
     return TARGET_XFER_E_IO;
 
+  int pid = get_ptrace_pid (inferior_ptid);
   errno = 0;
   ptrace (PTRACE_GETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, &siginfo);
   if (errno != 0)
@@ -4446,11 +4442,7 @@ linux_nat_target::linux_nat_target ()
 int
 linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo)
 {
-  int pid;
-
-  pid = ptid.lwp ();
-  if (pid == 0)
-    pid = ptid.pid ();
+  int pid = get_ptrace_pid (ptid);
 
   errno = 0;
   ptrace (PTRACE_GETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, siginfo);

base-commit: f9f593ddb2dee399e1ad24370c8e627aa4262524
-- 
2.38.1

[PATCH 2/6] gdb/linux-nat: bool-ify linux_nat_get_siginfo

[Simon Marchi]

Change return type to bool.

Change-Id: I1bf0360bfdd1b5994cd0f96c268d806f96fe51a4
---
 gdb/linux-nat.c | 6 +++---
 gdb/linux-nat.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9ab8b845cbb4..7b6a1f98d66d 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4439,7 +4439,7 @@ linux_nat_target::linux_nat_target ()
 
 /* See linux-nat.h.  */
 
-int
+bool
 linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo)
 {
   int pid = get_ptrace_pid (ptid);
@@ -4449,9 +4449,9 @@ linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo)
   if (errno != 0)
     {
       memset (siginfo, 0, sizeof (*siginfo));
-      return 0;
+      return false;
     }
-  return 1;
+  return true;
 }
 
 /* See nat/linux-nat.h.  */
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index a9b91a5e908b..576924395e70 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -330,8 +330,8 @@ extern void linux_unstop_all_lwps (void);
 void linux_nat_switch_fork (ptid_t new_ptid);
 
 /* Store the saved siginfo associated with PTID in *SIGINFO.
-   Return 1 if it was retrieved successfully, 0 otherwise (*SIGINFO is
+   Return true if it was retrieved successfully, false otherwise (*SIGINFO is
    uninitialized in such case).  */
-int linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo);
+bool linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo);
 
 #endif /* LINUX_NAT_H */
-- 
2.38.1

[PATCH 3/6] gdb/linux-nat: don't memset siginfo on failure in linux_nat_get_siginfo

[Simon Marchi]

No caller cares about the value of *SIGINFO on failure.  It's also
documented in the function doc that *SIGINFO is uninitialized (I
understand "untouched") on failure.

Change-Id: I5ef38a5f58e3635e109b919ddf6f827f38f1225a
---
 gdb/linux-nat.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 7b6a1f98d66d..c47b8db9299c 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4446,12 +4446,8 @@ linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo)
 
   errno = 0;
   ptrace (PTRACE_GETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, siginfo);
-  if (errno != 0)
-    {
-      memset (siginfo, 0, sizeof (*siginfo));
-      return false;
-    }
-  return true;
+
+  return errno == 0;
 }
 
 /* See nat/linux-nat.h.  */
-- 
2.38.1

[PATCH 4/6] gdb/linux-nat: check ptrace return value in linux_nat_get_siginfo

[Simon Marchi]

Not a big deal, but it seems strange to check errno instead of the
ptrace return value to know whether it succeeded.

Change-Id: If0a6d0280ab0e5ecb077e546af0d6fe489c5b9fd
---
 gdb/linux-nat.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index c47b8db9299c..b33fac3d1492 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4443,11 +4443,7 @@ bool
 linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo)
 {
   int pid = get_ptrace_pid (ptid);
-
-  errno = 0;
-  ptrace (PTRACE_GETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, siginfo);
-
-  return errno == 0;
+  return ptrace (PTRACE_GETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, siginfo) == 0;
 }
 
 /* See nat/linux-nat.h.  */
-- 
2.38.1

[PATCH 5/6] gdb/linux-nat: use l linux_nat_get_siginfo in linux_xfer_siginfo

[Simon Marchi]

I noticed we could reduce duplication a bit here.

Change-Id: If24e54d1dac71b46f7c1f68a18a073d4c084b644
---
 gdb/linux-nat.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b33fac3d1492..aeb81cd01bd2 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3620,10 +3620,7 @@ linux_xfer_siginfo (enum target_object object,
   if (offset > sizeof (siginfo))
     return TARGET_XFER_E_IO;
 
-  int pid = get_ptrace_pid (inferior_ptid);
-  errno = 0;
-  ptrace (PTRACE_GETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, &siginfo);
-  if (errno != 0)
+  if (!linux_nat_get_siginfo (inferior_ptid, &siginfo))
     return TARGET_XFER_E_IO;
 
   /* When GDB is built as a 64-bit application, ptrace writes into
@@ -3646,6 +3643,7 @@ linux_xfer_siginfo (enum target_object object,
       /* Convert back to ptrace layout before flushing it out.  */
       siginfo_fixup (&siginfo, inf_siginfo, 1);
 
+      int pid = get_ptrace_pid (inferior_ptid);
       errno = 0;
       ptrace (PTRACE_SETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, &siginfo);
       if (errno != 0)
-- 
2.38.1

[PATCH 6/6] gdb/linux-nat: add ptid parameter to linux_xfer_siginfo

[Simon Marchi]

Make the inferior_ptid bubble up to linux_nat_target::xfer_partial.

Change-Id: I62dbc5734c26648bb465f449c2003c73751cd812
---
 gdb/linux-nat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index aeb81cd01bd2..1d207c4e31d8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3606,7 +3606,7 @@ siginfo_fixup (siginfo_t *siginfo, gdb_byte *inf_siginfo, int direction)
 }
 
 static enum target_xfer_status
-linux_xfer_siginfo (enum target_object object,
+linux_xfer_siginfo (ptid_t ptid, enum target_object object,
 		    const char *annex, gdb_byte *readbuf,
 		    const gdb_byte *writebuf, ULONGEST offset, ULONGEST len,
 		    ULONGEST *xfered_len)
@@ -3620,7 +3620,7 @@ linux_xfer_siginfo (enum target_object object,
   if (offset > sizeof (siginfo))
     return TARGET_XFER_E_IO;
 
-  if (!linux_nat_get_siginfo (inferior_ptid, &siginfo))
+  if (!linux_nat_get_siginfo (ptid, &siginfo))
     return TARGET_XFER_E_IO;
 
   /* When GDB is built as a 64-bit application, ptrace writes into
@@ -3643,7 +3643,7 @@ linux_xfer_siginfo (enum target_object object,
       /* Convert back to ptrace layout before flushing it out.  */
       siginfo_fixup (&siginfo, inf_siginfo, 1);
 
-      int pid = get_ptrace_pid (inferior_ptid);
+      int pid = get_ptrace_pid (ptid);
       errno = 0;
       ptrace (PTRACE_SETSIGINFO, pid, (PTRACE_TYPE_ARG3) 0, &siginfo);
       if (errno != 0)
@@ -3672,7 +3672,7 @@ linux_nat_target::xfer_partial (enum target_object object,
 				ULONGEST offset, ULONGEST len, ULONGEST *xfered_len)
 {
   if (object == TARGET_OBJECT_SIGNAL_INFO)
-    return linux_xfer_siginfo (object, annex, readbuf, writebuf,
+    return linux_xfer_siginfo (inferior_ptid, object, annex, readbuf, writebuf,
 			       offset, len, xfered_len);
 
   /* The target is connected but no live inferior is selected.  Pass
-- 
2.38.1

Re: [PATCH 1/6] gdb/linux-nat: use get_ptrace_pid in two spots

[Tom Tromey]

Simon> No behavior change expected.

I read through these & they all look good to me.

Tom

Re: [PATCH 1/6] gdb/linux-nat: use get_ptrace_pid in two spots

[Simon Marchi]

On 12/5/22 15:39, Tom Tromey wrote:
> Simon> No behavior change expected.
> 
> I read through these & they all look good to me.
> 
> Tom

Thanks, pushed.

Simon