[PATCH 1/3] [gdbsupport] Add gdb::{waitpid,read,write,close}

[PATCH 1/3] [gdbsupport] Add gdb::{waitpid,read,write,close}

[Tom de Vries]

We have gdb::handle_eintr, which allows us to rewrite:
...
  ssize_t ret;
    do
      {
        errno = 0;
        ret = ::write (pipe[1], "+", 1);
      }
    while (ret == -1 && errno == EINTR);
...
into:
...
  ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
...

However, the call to write got a bit mangled, requiring effort to decode it
back to its original form.

Instead, add a new function gdb::write that allows us to write:
...
  ssize_t ret = gdb::write (pipe[1], "+", 1);
...

Likewise for waitpid, read and close.

Tested on x86_64-linux.
---
 gdb/cli/cli-cmds.c      |  2 +-
 gdb/nat/linux-waitpid.c |  2 +-
 gdbserver/netbsd-low.cc | 10 ++++----
 gdbsupport/Makefile.am  |  1 +
 gdbsupport/Makefile.in  |  1 +
 gdbsupport/eintr.cc     | 56 +++++++++++++++++++++++++++++++++++++++++
 gdbsupport/eintr.h      |  7 ++++++
 7 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 gdbsupport/eintr.cc

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 65ac7d6e7fb..230fcd549e1 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -923,7 +923,7 @@ run_under_shell (const char *arg, int from_tty)
 
   if (pid != -1)
     {
-      int ret = gdb::handle_eintr (-1, ::waitpid, pid, &status, 0);
+      int ret = gdb::waitpid (pid, &status, 0);
       if (ret == -1)
 	perror_with_name ("Cannot get status of shell command");
     }
diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index 0ac2f9fb2b9..f4ae612c572 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -51,5 +51,5 @@ status_to_str (int status)
 int
 my_waitpid (int pid, int *status, int flags)
 {
-  return gdb::handle_eintr (-1, ::waitpid, pid, status, flags);
+  return gdb::waitpid (pid, status, flags);
 }
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 4b58826e091..450fbcfb20d 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -222,7 +222,7 @@ netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
   int options = (target_options & TARGET_WNOHANG) ? WNOHANG : 0;
 
   pid_t pid
-    = gdb::handle_eintr (-1, ::waitpid, ptid.pid (), &status, options);
+    = gdb::waitpid (ptid.pid (), &status, options);
 
   if (pid == -1)
     perror_with_name (_("Child process unexpectedly missing"));
@@ -432,7 +432,7 @@ netbsd_process_target::kill (process_info *process)
     return -1;
 
   int status;
-  if (gdb::handle_eintr (-1, ::waitpid, pid, &status, 0) == -1)
+  if (gdb::waitpid (pid, &status, 0) == -1)
     return -1;
   mourn (process);
   return 0;
@@ -1123,15 +1123,15 @@ netbsd_qxfer_libraries_svr4 (const pid_t pid, const char *annex,
 static bool
 elf_64_file_p (const char *file)
 {
-  int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
+  int fd = gdb::open (file, O_RDONLY);
   if (fd < 0)
     perror_with_name (("open"));
 
   Elf64_Ehdr header;
-  ssize_t ret = gdb::handle_eintr (-1, ::read, fd, &header, sizeof (header));
+ ssize_t ret = gdb::read (fd, &header, sizeof (header));
   if (ret == -1)
     perror_with_name (("read"));
-  gdb::handle_eintr (-1, ::close, fd);
+  gdb::close (fd);
   if (ret != sizeof (header))
     error ("Cannot read ELF file header: %s", file);
 
diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
index e77298751cd..a3c3a11fdb2 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -61,6 +61,7 @@ libgdbsupport_a_SOURCES = \
     common-inferior.cc \
     common-regcache.cc \
     common-utils.cc \
+    eintr.cc \
     environ.cc \
     errors.cc \
     event-loop.cc \
diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in
index db3d6f6b4dd..a8696bca377 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -419,6 +419,7 @@ libgdbsupport_a_SOURCES = \
     common-inferior.cc \
     common-regcache.cc \
     common-utils.cc \
+    eintr.cc \
     environ.cc \
     errors.cc \
     event-loop.cc \
diff --git a/gdbsupport/eintr.cc b/gdbsupport/eintr.cc
new file mode 100644
index 00000000000..7e24fa56224
--- /dev/null
+++ b/gdbsupport/eintr.cc
@@ -0,0 +1,56 @@
+/* System calls wrappers for GDB, the GNU debugger.
+
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+namespace gdb {
+
+extern "C" {
+
+  pid_t
+  waitpid (pid_t pid, int *wstatus, int options)
+  {
+    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
+  }
+
+  int
+  open (const char *pathname, int flags)
+  {
+    return gdb::handle_eintr (-1, ::open, pathname, flags);
+  }
+
+  int
+  close (int fd)
+  {
+    return gdb::handle_eintr (-1, ::close, fd);
+  }
+
+  ssize_t
+  read (int fd, void *buf, size_t count)
+  {
+    return gdb::handle_eintr (-1, ::read, fd, buf, count);
+  }
+
+}
+
+}
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
index e440f935fcf..3919c9fd930 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -66,6 +66,13 @@ handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
   return ret;
 }
 
+extern "C" {
+  extern pid_t waitpid (pid_t pid, int *wstatus, int options);
+  extern int open (const char *pathname, int flags);
+  extern int close (int fd);
+  extern ssize_t read (int fd, void *buf, size_t count);
+}
+
 } /* namespace gdb */
 
 #endif /* GDBSUPPORT_EINTR_H */

base-commit: 77f6ff446173ac7790f35d43de7d196a768c38b1
-- 
2.35.3

[PATCH 2/3] [gdb] Use gdb::waitpid more often

[Tom de Vries]

Use gdb::waitpid instead of plain waitpid, making sure that EINTR is handled.

Tested on x86_64-linux.
---
 gdb/darwin-nat.c           |  3 ++-
 gdb/fbsd-nat.c             |  5 +++--
 gdb/inf-ptrace.c           | 13 +++++--------
 gdb/linux-fork.c           |  7 ++++---
 gdb/nat/linux-namespaces.c |  3 ++-
 gdb/nat/linux-ptrace.c     |  3 ++-
 gdb/netbsd-nat.c           |  9 +++------
 gdb/obsd-nat.c             |  3 ++-
 gdb/procfs.c               |  2 +-
 gdb/rs6000-aix-nat.c       |  9 +++------
 gdb/utils.c                |  5 +++--
 11 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 7ba1fbb6775..3cfcab895e1 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -70,6 +70,7 @@
 #include "gdbsupport/scoped_fd.h"
 #include "gdbsupport/scoped_restore.h"
 #include "nat/fork-inferior.h"
+#include "gdbsupport/eintr.h"
 
 /* Quick overview.
    Darwin kernel is Mach + BSD derived kernel.  Note that they share the
@@ -1604,7 +1605,7 @@ darwin_attach_pid (struct inferior *inf)
 	  if (!inf->attach_flag)
 	    {
 	      kill (inf->pid, 9);
-	      waitpid (inf->pid, &status, 0);
+	      gdb::waitpid (inf->pid, &status, 0);
 	    }
 
 	  error
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 6da32a4a50b..8e0523dd934 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -45,6 +45,7 @@
 #include "elf-bfd.h"
 #include "fbsd-nat.h"
 #include "fbsd-tdep.h"
+#include "gdbsupport/eintr.h"
 
 #ifndef PT_GETREGSET
 #define	PT_GETREGSET	42	/* Get a target register set */
@@ -1150,7 +1151,7 @@ fbsd_wait_for_fork_child (pid_t pid)
     return ptid;
 
   int status;
-  pid_t wpid = waitpid (pid, &status, 0);
+  pid_t wpid = gdb::waitpid (pid, &status, 0);
   if (wpid == -1)
     perror_with_name (("waitpid"));
 
@@ -2156,7 +2157,7 @@ fbsd_nat_target::kill ()
 	perror_with_name (("ptrace (PT_KILL)"));
 
   int status;
-  waitpid (pid, &status, 0);
+  gdb::waitpid (pid, &status, 0);
 
   target_mourn_inferior (inferior_ptid);
 }
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 36d6e2aa697..a910b5dd46e 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -32,6 +32,7 @@
 #include "nat/fork-inferior.h"
 #include "utils.h"
 #include "gdbarch.h"
+#include "gdbsupport/eintr.h"
 
 \f
 
@@ -122,7 +123,7 @@ inf_ptrace_target::mourn_inferior ()
      Do not check whether this succeeds though, since we may be
      dealing with a process that we attached to.  Such a process will
      only report its exit status to its original parent.  */
-  waitpid (inferior_ptid.pid (), &status, 0);
+  gdb::waitpid (inferior_ptid.pid (), &status, 0);
 
   inf_child_target::mourn_inferior ();
 }
@@ -227,7 +228,7 @@ inf_ptrace_target::kill ()
     return;
 
   ptrace (PT_KILL, pid, (PTRACE_TYPE_ARG3)0, 0);
-  waitpid (pid, &status, 0);
+  gdb::waitpid (pid, &status, 0);
 
   target_mourn_inferior (inferior_ptid);
 }
@@ -307,12 +308,8 @@ inf_ptrace_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
     {
       set_sigint_trap ();
 
-      do
-	{
-	  pid = waitpid (ptid.pid (), &status, options);
-	  save_errno = errno;
-	}
-      while (pid == -1 && errno == EINTR);
+      pid = gdb::waitpid (ptid.pid (), &status, options);
+      save_errno = errno;
 
       clear_sigint_trap ();
 
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index c457a90556d..7d3505b5948 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -32,6 +32,7 @@
 
 #include "nat/gdb_ptrace.h"
 #include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/eintr.h"
 #include "target/waitstatus.h"
 #include <dirent.h>
 #include <ctype.h>
@@ -314,7 +315,7 @@ linux_fork_killall (void)
 	/* Use SIGKILL instead of PTRACE_KILL because the former works even
 	   if the thread is running, while the later doesn't.  */
 	kill (pid, SIGKILL);
-	ret = waitpid (pid, &status, 0);
+	ret = gdb::waitpid (pid, &status, 0);
 	/* We might get a SIGCHLD instead of an exit status.  This is
 	 aggravated by the first kill above - a child has just
 	 died.  MVS comment cut-and-pasted from linux-nat.  */
@@ -339,7 +340,7 @@ linux_fork_mourn_inferior (void)
      Do not check whether this succeeds though, since we may be
      dealing with a process that we attached to.  Such a process will
      only report its exit status to its original parent.  */
-  waitpid (inferior_ptid.pid (), &status, 0);
+  gdb::waitpid (inferior_ptid.pid (), &status, 0);
 
   /* OK, presumably inferior_ptid is the one who has exited.
      We need to delete that one from the fork_list, and switch
@@ -548,7 +549,7 @@ Please switch to another checkpoint before deleting the current one"));
 	 this succeeds though, since we may be dealing with a process that we
 	 attached to.  Such a process will only report its exit status to its
 	 original parent.  */
-      waitpid (ptid.pid (), &status, 0);
+      gdb::waitpid (ptid.pid (), &status, 0);
       return;
     }
 
diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c
index 9abd3d689ea..19a05eec905 100644
--- a/gdb/nat/linux-namespaces.c
+++ b/gdb/nat/linux-namespaces.c
@@ -28,6 +28,7 @@
 #include <signal.h>
 #include <sched.h>
 #include "gdbsupport/scope-exit.h"
+#include "gdbsupport/eintr.h"
 
 /* See nat/linux-namespaces.h.  */
 bool debug_linux_namespaces;
@@ -722,7 +723,7 @@ mnsh_maybe_mourn_peer (void)
 	  return;
 	}
 
-      pid = waitpid (helper->pid, &status, WNOHANG);
+      pid = gdb::waitpid (helper->pid, &status, WNOHANG);
       if (pid == 0)
 	{
 	  /* The helper is still alive.  */
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 9ea0e22913f..f90183b63ec 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -22,6 +22,7 @@
 #ifdef HAVE_SYS_PROCFS_H
 #include <sys/procfs.h>
 #endif
+#include "gdbsupport/eintr.h"
 
 /* Stores the ptrace options supported by the running kernel.
    A value of -1 means we did not check for features yet.  A value
@@ -177,7 +178,7 @@ linux_ptrace_test_ret_to_nx (void)
     }
 
   errno = 0;
-  got_pid = waitpid (child, &status, 0);
+  got_pid = gdb::waitpid (child, &status, 0);
   if (got_pid != child)
     {
       warning (_("linux_ptrace_test_ret_to_nx: waitpid returned %ld: %s"),
diff --git a/gdb/netbsd-nat.c b/gdb/netbsd-nat.c
index 90456de165e..5005cb8307c 100644
--- a/gdb/netbsd-nat.c
+++ b/gdb/netbsd-nat.c
@@ -25,6 +25,7 @@
 #include "inferior.h"
 #include "gdbarch.h"
 #include "gdbsupport/buildargv.h"
+#include "gdbsupport/eintr.h"
 
 #include <sys/types.h>
 #include <sys/ptrace.h>
@@ -547,12 +548,8 @@ nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 
   set_sigint_trap ();
 
-  do
-    {
-      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
-      pid = waitpid (ptid.pid (), &status, 0);
-    }
-  while (pid == -1 && errno == EINTR);
+  /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
+  pid = gdb::waitpid (ptid.pid (), &status, 0);
 
   clear_sigint_trap ();
 
diff --git a/gdb/obsd-nat.c b/gdb/obsd-nat.c
index 69a21ad71fc..701a3653d32 100644
--- a/gdb/obsd-nat.c
+++ b/gdb/obsd-nat.c
@@ -27,6 +27,7 @@
 
 #include "inf-ptrace.h"
 #include "obsd-nat.h"
+#include "gdbsupport/eintr.h"
 
 /* OpenBSD 5.2 and later include rthreads which uses a thread model
    that maps userland threads directly onto kernel threads in a 1:1
@@ -105,7 +106,7 @@ obsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	  ourstatus->set_forked (ptid_t (pe.pe_other_pid));
 
 	  /* Make sure the other end of the fork is stopped too.  */
-	  pid_t fpid = waitpid (pe.pe_other_pid, nullptr, 0);
+	  pid_t fpid = gdb::waitpid (pe.pe_other_pid, nullptr, 0);
 	  if (fpid == -1)
 	    perror_with_name (("waitpid"));
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index c6abe3ead44..22e012f0f31 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2560,7 +2560,7 @@ unconditionally_kill_inferior (procinfo *pi)
 #if 0
       int status, ret;
 
-      ret = waitpid (pi->pid, &status, 0);
+      ret = gdb::waitpid (pi->pid, &status, 0);
 #else
       wait (NULL);
 #endif
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 6a20f612413..674189b0771 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -42,6 +42,7 @@
 #include <signal.h>
 #include <sys/ioctl.h>
 #include <fcntl.h>
+#include "gdbsupport/eintr.h"
 
 #include <a.out.h>
 #include <sys/file.h>
@@ -865,12 +866,8 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
     {
       set_sigint_trap ();
 
-      do
-	{
-	  pid = waitpid (ptid.pid (), &status, 0);
-	  save_errno = errno;
-	}
-      while (pid == -1 && errno == EINTR);
+      pid = gdb::waitpid (ptid.pid (), &status, 0);
+      save_errno = errno;
 
       clear_sigint_trap ();
 
diff --git a/gdb/utils.c b/gdb/utils.c
index a1bf9e46e02..ea98566dd3b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -81,6 +81,7 @@
 #include "pager.h"
 #include "run-on-main-thread.h"
 #include "gdbsupport/gdb_tilde_expand.h"
+#include "gdbsupport/eintr.h"
 
 void (*deprecated_error_begin_hook) (void);
 
@@ -3483,7 +3484,7 @@ wait_to_die_with_timeout (pid_t pid, int *status, int timeout)
       alarm (timeout);
 #endif
 
-      waitpid_result = waitpid (pid, status, 0);
+      waitpid_result = gdb::waitpid (pid, status, 0);
 
 #ifdef SIGALRM
       alarm (0);
@@ -3495,7 +3496,7 @@ wait_to_die_with_timeout (pid_t pid, int *status, int timeout)
 #endif
     }
   else
-    waitpid_result = waitpid (pid, status, WNOHANG);
+    waitpid_result = gdb::waitpid (pid, status, WNOHANG);
 
   if (waitpid_result == pid)
     return pid;
-- 
2.35.3

[PATCH 3/3] [gdbsupport] Add gdb::wait

[Tom de Vries]

Add gdb::wait, and use it in gdb/procfs.c, making sure that EINTR is handled.

Tested on x86_64-linux.
---
 gdb/procfs.c        | 9 +++++----
 gdbsupport/eintr.cc | 6 ++++++
 gdbsupport/eintr.h  | 2 ++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/gdb/procfs.c b/gdb/procfs.c
index 22e012f0f31..d5177f3735d 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2062,8 +2062,9 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status,
 	    {
 	      int wait_retval;
 
-	      /* /proc file not found; presumably child has terminated.  */
-	      wait_retval = ::wait (&wstat); /* "wait" for the child's exit.  */
+	      /* /proc file not found; presumably child has terminated.  Wait
+		 for the child's exit.  */
+	      wait_retval = gdb::wait (&wstat);
 
 	      /* Wrong child?  */
 	      if (wait_retval != inf->pid)
@@ -2150,7 +2151,7 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status,
 		      }
 		    else
 		      {
-			int temp = ::wait (&wstat);
+			int temp = gdb::wait (&wstat);
 
 			/* FIXME: shouldn't I make sure I get the right
 			   event from the right process?  If (for
@@ -2562,7 +2563,7 @@ unconditionally_kill_inferior (procinfo *pi)
 
       ret = gdb::waitpid (pi->pid, &status, 0);
 #else
-      wait (NULL);
+      gdb::wait (NULL);
 #endif
     }
 }
diff --git a/gdbsupport/eintr.cc b/gdbsupport/eintr.cc
index 7e24fa56224..cb6e45feb3c 100644
--- a/gdbsupport/eintr.cc
+++ b/gdbsupport/eintr.cc
@@ -33,6 +33,12 @@ extern "C" {
     return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
   }
 
+  pid_t
+  wait (int *wstatus)
+  {
+    return gdb::handle_eintr (-1, ::wait, wstatus);
+  }
+
   int
   open (const char *pathname, int flags)
   {
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
index 3919c9fd930..286da5fc400 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -68,6 +68,8 @@ handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
 
 extern "C" {
   extern pid_t waitpid (pid_t pid, int *wstatus, int options);
+  extern pid_t wait (int *wstatus);
+
   extern int open (const char *pathname, int flags);
   extern int close (int fd);
   extern ssize_t read (int fd, void *buf, size_t count);
-- 
2.35.3

Re: [PATCH 1/3] [gdbsupport] Add gdb::{waitpid,read,write,close}

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +namespace gdb {
Tom> +
Tom> +extern "C" {

I didn't even know this was valid ... but is it really needed?
It seems wrong.

Tom> +  pid_t
Tom> +  waitpid (pid_t pid, int *wstatus, int options)
Tom> +  {
Tom> +    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
Tom> +  }

Also it seems like these could all be inline functions in the header
anyway.

Tom

Re: [PATCH 1/3] [gdbsupport] Add gdb::{waitpid,read,write,close}

[Tom de Vries]

On 11/1/24 18:35, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> +namespace gdb {
> Tom> +
> Tom> +extern "C" {
> 
> I didn't even know this was valid ... but is it really needed?
> It seems wrong.
> 

It seemed to work:
...
$ cat test.c
namespace gdb {

   extern "C" {
     int foo (void) { return 1; };
   }
}

$ cat test2.c
namespace gdb {
   extern "C" {
     int foo (void);
   }
}

int
main (void)
{
   return gdb::foo ();
}
$ g++ test.c test2.c
...
but the linkage symbol is plain foo:
...
$ nm ./a.out  | grep foo
00000000004004c7 T foo
...

So, you're right, this is wrong.

I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, 
similar to gdb_select, but thinking about it now I slightly prefer 
gdb::waitpid.  I suppose it doesn't matter that much.

> Tom> +  pid_t
> Tom> +  waitpid (pid_t pid, int *wstatus, int options)
> Tom> +  {
> Tom> +    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, options);
> Tom> +  }
> 
> Also it seems like these could all be inline functions in the header
> anyway.

Done in v2.  In the case of fnctl, I ended up with a template to handle 
the variable number of arguments.

Submitted here ( 
https://sourceware.org/pipermail/gdb-patches/2024-November/212941.html ).

Thanks,
- Tom

Re: [PATCH 1/3] [gdbsupport] Add gdb::{waitpid,read,write,close}

[Tom de Vries]

On 11/5/24 13:07, Tom de Vries wrote:
> On 11/1/24 18:35, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> +namespace gdb {
>> Tom> +
>> Tom> +extern "C" {
>>
>> I didn't even know this was valid ... but is it really needed?
>> It seems wrong.
>>
> 
> It seemed to work:
> ...
> $ cat test.c
> namespace gdb {
> 
>    extern "C" {
>      int foo (void) { return 1; };
>    }
> }
> 
> $ cat test2.c
> namespace gdb {
>    extern "C" {
>      int foo (void);
>    }
> }
> 
> int
> main (void)
> {
>    return gdb::foo ();
> }
> $ g++ test.c test2.c
> ...
> but the linkage symbol is plain foo:
> ...
> $ nm ./a.out  | grep foo
> 00000000004004c7 T foo
> ...
> 
> So, you're right, this is wrong.
> 
> I seem to have ended up with gdb_waitpid instead of gdb::waitpid in v2, 
> similar to gdb_select, but thinking about it now I slightly prefer 
> gdb::waitpid.  I suppose it doesn't matter that much.
> 

When looking at this again today, it bugged me enough rewrite v2 in the 
gdb::waitpid style.  Committed after retesting.

Thanks,
- Tom

>> Tom> +  pid_t
>> Tom> +  waitpid (pid_t pid, int *wstatus, int options)
>> Tom> +  {
>> Tom> +    return gdb::handle_eintr (-1, ::waitpid, pid, wstatus, 
>> options);
>> Tom> +  }
>>
>> Also it seems like these could all be inline functions in the header
>> anyway.
> 
> Done in v2.  In the case of fnctl, I ended up with a template to handle 
> the variable number of arguments.
> 
> Submitted here ( https://sourceware.org/pipermail/gdb-patches/2024- 
> November/212941.html ).
> 
> Thanks,
> - Tom
>