[PATCH 00/10] Refactor the NetBSD gdbserver support

[PATCH 00/10] Refactor the NetBSD gdbserver support

[Kamil Rytarowski]

Changes:

 * Turn local static functions into the netbsd_process_target class members.
 * Remove netbsd_tdesc and netbsd_add_process as superfluous.
 * Merge netbsd_qxfer_libraries_svr4 into
   netbsd_process_target::qxfer_libraries_svr4.
 * While there, fix whitespace formatting.

No functional change intended to the code.

Kamil Rytarowski (10):
  Merge netbsd_ptrace_fun into the netbsd_process_target class
  Include elf_64_file_p in the netbsd_process_target class
  Remove unneeded netbsd_add_process()
  Include gdb_catching_syscalls_p in the netbsd_process_target class
  Include netbsd_wait in the netbsd_process_target class
  Include netbsd_waitpid in the netbsd_process_target class
  Include netbsd_store_waitstatus in the netbsd_process_target class
  Include netbsd_catch_this_syscall in the netbsd_process_target class
  Include the functions of qxfer_libraries_svr4 in netbsd_process_target
  Fix whitespace formatting

 gdbserver/ChangeLog           |  70 +++++++++
 gdbserver/netbsd-amd64-low.cc |  10 +-
 gdbserver/netbsd-low.cc       | 288 +++++++++++++++-------------------
 gdbserver/netbsd-low.h        |  49 ++++++
 4 files changed, 250 insertions(+), 167 deletions(-)

--
2.28.0

[PATCH 01/10] Merge netbsd_ptrace_fun into the netbsd_process_target class

[Kamil Rytarowski]

Change netbsd_ptrace_fun into a local lambda function, as otherwise
if it is a class member it is harder to pass it as a callbakc function
to the fork_inferior call.

No functional change.

gdb/ChangeLog:

	* netbsd-low.cc (netbsd_ptrace_fun): Turn into...
	(netbsd_process_target::create_inferior): ...lamda here.
---
 gdbserver/ChangeLog     |  5 ++++
 gdbserver/netbsd-low.cc | 65 ++++++++++++++++++++---------------------
 2 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index d667a203e67..94fb3409f23 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (netbsd_ptrace_fun): Turn into...
+	(netbsd_process_target::create_inferior): ...lamda here.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-i386-low.cc: Add.
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 7bec55a56ac..8a7ca5294f0 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -56,38 +56,6 @@ netbsd_add_process (int pid, int attached)
   proc->priv = nullptr;
 }

-/* Callback used by fork_inferior to start tracing the inferior.  */
-
-static void
-netbsd_ptrace_fun ()
-{
-  /* Switch child to its own process group so that signals won't
-     directly affect GDBserver. */
-  if (setpgid (0, 0) < 0)
-    trace_start_error_with_name (("setpgid"));
-
-  if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
-    trace_start_error_with_name (("ptrace"));
-
-  /* If GDBserver is connected to gdb via stdio, redirect the inferior's
-     stdout to stderr so that inferior i/o doesn't corrupt the connection.
-     Also, redirect stdin to /dev/null.  */
-  if (remote_connection_is_stdio ())
-    {
-      if (close (0) < 0)
-	trace_start_error_with_name (("close"));
-      if (open ("/dev/null", O_RDONLY) < 0)
-	trace_start_error_with_name (("open"));
-      if (dup2 (2, 1) < 0)
-	trace_start_error_with_name (("dup2"));
-      if (write (2, "stdin/stdout redirected\n",
-		 sizeof ("stdin/stdout redirected\n") - 1) < 0)
-	{
-	  /* Errors ignored.  */
-	}
-    }
-}
-
 /* Implement the create_inferior method of the target_ops vector.  */

 int
@@ -96,8 +64,39 @@ netbsd_process_target::create_inferior (const char *program,
 {
   std::string str_program_args = construct_inferior_arguments (program_args);

+  /* Callback used by fork_inferior to start tracing the inferior.  */
+  auto fn
+    = [] ()
+      {
+	/* Switch child to its own process group so that signals won't
+	   directly affect GDBserver. */
+	if (setpgid (0, 0) < 0)
+	  trace_start_error_with_name (("setpgid"));
+
+	if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
+	  trace_start_error_with_name (("ptrace"));
+
+	/* If GDBserver is connected to gdb via stdio, redirect the inferior's
+	   stdout to stderr so that inferior i/o doesn't corrupt the connection.
+	   Also, redirect stdin to /dev/null.  */
+	if (remote_connection_is_stdio ())
+	  {
+	    if (close (0) < 0)
+	      trace_start_error_with_name (("close"));
+	    if (open ("/dev/null", O_RDONLY) < 0)
+	      trace_start_error_with_name (("open"));
+	    if (dup2 (2, 1) < 0)
+	      trace_start_error_with_name (("dup2"));
+	    if (write (2, "stdin/stdout redirected\n",
+		       sizeof ("stdin/stdout redirected\n") - 1) < 0)
+	      {
+		/* Errors ignored.  */
+	      }
+	  }
+      };
+
   pid_t pid = fork_inferior (program, str_program_args.c_str (),
-			     get_environ ()->envp (), netbsd_ptrace_fun,
+			     get_environ ()->envp (), fn,
 			     nullptr, nullptr, nullptr, nullptr);

   netbsd_add_process (pid, 0);
--
2.28.0

[PATCH 02/10] Include elf_64_file_p in the netbsd_process_target class

[Kamil Rytarowski]

gdbserver/ChangeLog:

        * netbsd-low.cc (elf_64_file_p): Turn into...
        (netbsd_process_target::elf_64_file_p): ...this.
        * netbsd-low.h (netbsd_process_target::elf_64_file_p): Add.
---
 gdbserver/ChangeLog     | 6 ++++++
 gdbserver/netbsd-low.cc | 8 +++-----
 gdbserver/netbsd-low.h  | 8 ++++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 94fb3409f23..47714e8f85a 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (elf_64_file_p): Turn into...
+	(netbsd_process_target::elf_64_file_p): ...this.
+	* netbsd-low.h (netbsd_process_target::elf_64_file_p): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (netbsd_ptrace_fun): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 8a7ca5294f0..b1f2454f755 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -1197,12 +1197,10 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
   return len;
 }

-/* Return true if FILE is a 64-bit ELF file,
-   false if the file is not a 64-bit ELF file,
-   and error if the file is not accessible or doesn't exist.  */
+/* See netbsd-low.h.  */

-static bool
-elf_64_file_p (const char *file)
+bool
+netbsd_process_target::elf_64_file_p (const char *file)
 {
   int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
   if (fd < 0)
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index c229a0f9f61..0d18e329b59 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -127,6 +127,14 @@ class netbsd_process_target : public process_stratum_target

   bool supports_catch_syscall () override;

+protected:
+  /* The architecture-independent NetBSD specific methods are listed below.  */
+
+  /* Return true if FILE is a 64-bit ELF file,
+     false if the file is not a 64-bit ELF file,
+     and error if the file is not accessible or doesn't exist.  */
+  bool elf_64_file_p (const char *file);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 03/10] Remove unneeded netbsd_add_process()

[Kamil Rytarowski]

Currently it does not add any value.

The netbsd_tdesc local variable is no longer needed. Remove it.
The tdesc value is set by the low target now.

gdbserver/ChangeLog:

        * netbsd-low.cc (netbsd_tdesc): Remove.
        (netbsd_add_process): Likewise.
        (netbsd_process_target::create_inferior): Update.
---
 gdbserver/ChangeLog     |  6 ++++++
 gdbserver/netbsd-low.cc | 15 +--------------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 47714e8f85a..4fa59ae2dad 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (netbsd_tdesc): Remove.
+	(netbsd_add_process): Likewise.
+	(netbsd_process_target::create_inferior): Update.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (elf_64_file_p): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index b1f2454f755..c2f34bd3339 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -43,19 +43,6 @@

 int using_threads = 1;

-const struct target_desc *netbsd_tdesc;
-
-/* Call add_process with the given parameters, and initialize
-   the process' private data.  */
-
-static void
-netbsd_add_process (int pid, int attached)
-{
-  struct process_info *proc = add_process (pid, attached);
-  proc->tdesc = netbsd_tdesc;
-  proc->priv = nullptr;
-}
-
 /* Implement the create_inferior method of the target_ops vector.  */

 int
@@ -99,7 +86,7 @@ netbsd_process_target::create_inferior (const char *program,
 			     get_environ ()->envp (), fn,
 			     nullptr, nullptr, nullptr, nullptr);

-  netbsd_add_process (pid, 0);
+  add_process (pid, 0);

   post_fork_inferior (pid, program);

--
2.28.0

[PATCH 04/10] Include gdb_catching_syscalls_p in the netbsd_process_target class

[Kamil Rytarowski]

gdb/ChangeLog:

        * netbsd-low.cc (netbsd_tdesc): Remove.
        (netbsd_add_process): Likewise.
        (netbsd_process_target::create_inferior): Update.
---
 gdbserver/ChangeLog     | 6 ++++++
 gdbserver/netbsd-low.cc | 6 +++---
 gdbserver/netbsd-low.h  | 3 +++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 4fa59ae2dad..17f78193be9 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (gdb_catching_syscalls_p): Turn into...
+	(netbsd_process_target::gdb_catching_syscalls_p): ...this.
+	* netbsd-low.h (netbsd_process_target::gdb_catching_syscalls_p): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (netbsd_tdesc): Remove.
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index c2f34bd3339..bcae3a6f757 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -113,10 +113,10 @@ netbsd_process_target::attach (unsigned long pid)
   return -1;
 }

-/* Returns true if GDB is interested in any child syscalls.  */
+/* See netbsd-low.h.  */

-static bool
-gdb_catching_syscalls_p (pid_t pid)
+bool
+netbsd_process_target::gdb_catching_syscalls_p (pid_t pid)
 {
   struct process_info *proc = find_process_pid (pid);
   return !proc->syscalls_to_catch.empty ();
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index 0d18e329b59..6797868dbe7 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -135,6 +135,9 @@ class netbsd_process_target : public process_stratum_target
      and error if the file is not accessible or doesn't exist.  */
   bool elf_64_file_p (const char *file);

+  /* Returns true if GDB is interested in any child syscalls.  */
+  bool gdb_catching_syscalls_p (pid_t pid);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 05/10] Include netbsd_wait in the netbsd_process_target class

[Kamil Rytarowski]

gdbserver/ChangeLog:

       * netbsd-low.cc (netbsd_wait): Turn into...
       (netbsd_process_target::netbsd_wait): ...this.
       * netbsd-low.h (netbsd_process_target::netbsd_wait): Add.
---
 gdbserver/ChangeLog     |  6 ++++++
 gdbserver/netbsd-low.cc | 12 ++++--------
 gdbserver/netbsd-low.h  |  6 ++++++
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 17f78193be9..f978ac34fcd 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (netbsd_wait): Turn into...
+	(netbsd_process_target::netbsd_wait): ...this.
+	* netbsd-low.h (netbsd_process_target::netbsd_wait): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (gdb_catching_syscalls_p): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index bcae3a6f757..c834b5c6f78 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -241,15 +241,11 @@ netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
 }


-/* Implement the wait target_ops method.
-
-   Wait for the child specified by PTID to do something.  Return the
-   process ID of the child, or MINUS_ONE_PTID in case of error; store
-   the status in *OURSTATUS.  */
+/* See netbsd-low.h.  */

-static ptid_t
-netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
-	     target_wait_flags target_options)
+ptid_t
+netbsd_process_target::netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+				    target_wait_flags target_options)
 {
   pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);
   ptid_t wptid = ptid_t (pid);
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index 6797868dbe7..cb4a3ce44ed 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -138,6 +138,12 @@ class netbsd_process_target : public process_stratum_target
   /* Returns true if GDB is interested in any child syscalls.  */
   bool gdb_catching_syscalls_p (pid_t pid);

+  /* Wait for the child specified by PTID to do something.  Return the
+     process ID of the child, or MINUS_ONE_PTID in case of error; store
+     the status in *OURSTATUS.  */
+  ptid_t netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+		      target_wait_flags target_options);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 06/10] Include netbsd_waitpid in the netbsd_process_target class

[Kamil Rytarowski]

gdbserver/ChangeLog:

       * netbsd-low.cc (netbsd_waitpid): Turn into...
       (netbsd_process_target::netbsd_waitpid): ...this.
       * netbsd-low.h (netbsd_process_target::netbsd_waitpid): Add.
---
 gdbserver/ChangeLog     | 6 ++++++
 gdbserver/netbsd-low.cc | 8 ++++----
 gdbserver/netbsd-low.h  | 4 ++++
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index f978ac34fcd..d80c77c16e3 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (netbsd_waitpid): Turn into...
+	(netbsd_process_target::netbsd_waitpid): ...this.
+	* netbsd-low.h (netbsd_process_target::netbsd_waitpid): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (netbsd_wait): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index c834b5c6f78..f4dd3857495 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -221,11 +221,11 @@ netbsd_store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
     }
 }

-/* Implement a safe wrapper around waitpid().  */
+/* See netbsd-low.h.  */

-static pid_t
-netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
-		target_wait_flags target_options)
+pid_t
+netbsd_process_target::netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
+				       target_wait_flags target_options)
 {
   int status;
   int options = (target_options & TARGET_WNOHANG) ? WNOHANG : 0;
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index cb4a3ce44ed..8df03c39d03 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -144,6 +144,10 @@ class netbsd_process_target : public process_stratum_target
   ptid_t netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		      target_wait_flags target_options);

+  /* Implement a safe wrapper around waitpid().  */
+  pid_t netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
+			target_wait_flags target_options);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 07/10] Include netbsd_store_waitstatus in the netbsd_process_target class

[Kamil Rytarowski]

gdbserver/ChangeLog:

       * netbsd-low.cc (netbsd_store_waitstatus): Turn into...
       (netbsd_process_target::netbsd_store_waitstatus): ...this.
       * netbsd-low.h (netbsd_process_target::netbsd_store_waitstatus): Add.
---
 gdbserver/ChangeLog     | 6 ++++++
 gdbserver/netbsd-low.cc | 9 ++++-----
 gdbserver/netbsd-low.h  | 5 +++++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index d80c77c16e3..aabfb23b932 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (netbsd_store_waitstatus): Turn into...
+	(netbsd_process_target::netbsd_store_waitstatus): ...this.
+	* netbsd-low.h (netbsd_process_target::netbsd_store_waitstatus): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (netbsd_waitpid): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index f4dd3857495..bc198f0a7c8 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -197,12 +197,11 @@ netbsd_catch_this_syscall (int sysno)
   return false;
 }

-/* Helper function for child_wait and the derivatives of child_wait.
-   HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
-   translation of that in OURSTATUS.  */
+/* See netbsd-low.h.  */

-static void
-netbsd_store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
+void
+netbsd_process_target::netbsd_store_waitstatus (target_waitstatus *ourstatus,
+						int hoststatus)
 {
   if (WIFEXITED (hoststatus))
     {
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index 8df03c39d03..1458dd759eb 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -148,6 +148,11 @@ class netbsd_process_target : public process_stratum_target
   pid_t netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
 			target_wait_flags target_options);

+  /* Helper function for child_wait and the derivatives of child_wait.
+     HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
+     translation of that in OURSTATUS.  */
+  void netbsd_store_waitstatus (target_waitstatus *ourstatus, int hoststatus);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 08/10] Include netbsd_catch_this_syscall in the netbsd_process_target class

[Kamil Rytarowski]

gdbserver/ChangeLog:

       * netbsd-low.cc (netbsd_catch_this_syscall): Turn into...
       (netbsd_process_target::netbsd_catch_this_syscall): ...this.
       * netbsd-low.h (netbsd_process_target::netbsd_catch_this_syscall): Add.
---
 gdbserver/ChangeLog     | 6 ++++++
 gdbserver/netbsd-low.cc | 6 +++---
 gdbserver/netbsd-low.h  | 3 +++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index aabfb23b932..d81d961432a 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (netbsd_catch_this_syscall): Turn into...
+	(netbsd_process_target::netbsd_catch_this_syscall): ...this.
+	* netbsd-low.h (netbsd_process_target::netbsd_catch_this_syscall): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (netbsd_store_waitstatus): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index bc198f0a7c8..9a55bd42713 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -177,10 +177,10 @@ netbsd_process_target::resume (struct thread_resume *resume_info, size_t n)
     perror_with_name (("ptrace"));
 }

-/* Returns true if GDB is interested in the reported SYSNO syscall.  */
+/* See netbsd-low.h.  */

-static bool
-netbsd_catch_this_syscall (int sysno)
+bool
+netbsd_process_target::netbsd_catch_this_syscall (int sysno)
 {
   struct process_info *proc = current_process ();

diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index 1458dd759eb..7ed08dc90fa 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -153,6 +153,9 @@ class netbsd_process_target : public process_stratum_target
      translation of that in OURSTATUS.  */
   void netbsd_store_waitstatus (target_waitstatus *ourstatus, int hoststatus);

+  /* Returns true if GDB is interested in the reported SYSNO syscall.  */
+  bool netbsd_catch_this_syscall (int sysno);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 09/10] Include the functions of qxfer_libraries_svr4 in netbsd_process_target

[Kamil Rytarowski]

Merge get_phdr_phnum_from_proc_auxv, get_dynamic, get_r_debug and read_one_ptr
in the netbsd_process_target class. All of them are called indirectly
from the public target method qxfer_libraries_svr4.

For code simplicity, merge netbsd_qxfer_libraries_svr4 into
netbsd_process_target::qxfer_libraries_svr4.

gdbserver/ChangeLog:

        * netbsd-low.cc (get_phdr_phnum_from_proc_auxv): Turn into...
        (netbsd_process_target::get_phdr_phnum_from_proc_auxv): ...this.
        (get_dynamic): Turn into...
        (netbsd_process_target::get_dynamic): ...this.
        (get_r_debug): Turn into...
        (netbsd_process_target::get_r_debug): ...this.
        (read_one_ptr): Turn into...
        (netbsd_process_target::read_one_ptr): ...this.
        (netbsd_qxfer_libraries_svr4): Merge into...
        (netbsd_process_target::qxfer_libraries_svr4): ...this.
        * netbsd-low.h
        (netbsd_process_target::get_phdr_phnum_from_proc_auxv)
        (netbsd_process_target::get_dynamic)
        (netbsd_process_target::get_r_debug)
        (netbsd_process_target::read_one_ptr): Add.
---
 gdbserver/ChangeLog     |  18 +++++
 gdbserver/netbsd-low.cc | 161 ++++++++++++++++++----------------------
 gdbserver/netbsd-low.h  |  20 +++++
 3 files changed, 111 insertions(+), 88 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index d81d961432a..0c1c40e0581 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,21 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc (get_phdr_phnum_from_proc_auxv): Turn into...
+	(netbsd_process_target::get_phdr_phnum_from_proc_auxv): ...this.
+	(get_dynamic): Turn into...
+	(netbsd_process_target::get_dynamic): ...this.
+	(get_r_debug): Turn into...
+	(netbsd_process_target::get_r_debug): ...this.
+	(read_one_ptr): Turn into...
+	(netbsd_process_target::read_one_ptr): ...this.
+	(netbsd_qxfer_libraries_svr4): Merge into...
+	(netbsd_process_target::qxfer_libraries_svr4): ...this.
+	* netbsd-low.h
+	(netbsd_process_target::get_phdr_phnum_from_proc_auxv)
+	(netbsd_process_target::get_dynamic)
+	(netbsd_process_target::get_r_debug)
+	(netbsd_process_target::read_one_ptr): Add.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (netbsd_catch_this_syscall): Turn into...
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 9a55bd42713..f1011cdfe73 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -798,11 +798,13 @@ netbsd_process_target::supports_disable_randomization ()
   return false;
 }

-/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
+/* See netbsd-low.h.  */

 template <typename T>
-int get_phdr_phnum_from_proc_auxv (const pid_t pid,
-				   CORE_ADDR *phdr_memaddr, int *num_phdr)
+int
+netbsd_process_target::get_phdr_phnum_from_proc_auxv (const pid_t pid,
+						      CORE_ADDR *phdr_memaddr,
+						      int *num_phdr)
 {
   typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
 				    Aux64Info, Aux32Info>::type auxv_type;
@@ -848,11 +850,11 @@ int get_phdr_phnum_from_proc_auxv (const pid_t pid,
   return 0;
 }

-/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
+/* See netbsd-low.h.  */

 template <typename T>
-static CORE_ADDR
-get_dynamic (netbsd_process_target *target, const pid_t pid)
+CORE_ADDR
+netbsd_process_target::get_dynamic (const pid_t pid)
 {
   typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
 				    Elf64_Phdr, Elf32_Phdr>::type phdr_type;
@@ -866,7 +868,7 @@ get_dynamic (netbsd_process_target *target, const pid_t pid)
   std::vector<unsigned char> phdr_buf;
   phdr_buf.resize (num_phdr * phdr_size);

-  if (target->read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))
+  if (read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))
     return 0;

   /* Compute relocation: it is expected to be 0 for "regular" executables,
@@ -906,14 +908,11 @@ get_dynamic (netbsd_process_target *target, const pid_t pid)
   return 0;
 }

-/* Return &_r_debug in the inferior, or -1 if not present.  Return value
-   can be 0 if the inferior does not yet have the library list initialized.
-   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
-   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
+/* See netbsd-low.h.  */

 template <typename T>
-static CORE_ADDR
-get_r_debug (netbsd_process_target *target, const int pid)
+CORE_ADDR
+netbsd_process_target::get_r_debug (const int pid)
 {
   typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
 				    Elf64_Dyn, Elf32_Dyn>::type dyn_type;
@@ -921,11 +920,11 @@ get_r_debug (netbsd_process_target *target, const int pid)
   unsigned char buf[sizeof (dyn_type)];  /* The larger of the two.  */
   CORE_ADDR map = -1;

-  CORE_ADDR dynamic_memaddr = get_dynamic<T> (target, pid);
+  CORE_ADDR dynamic_memaddr = get_dynamic<T> (pid);
   if (dynamic_memaddr == 0)
     return map;

-  while (target->read_memory (dynamic_memaddr, buf, dyn_size) == 0)
+  while (read_memory (dynamic_memaddr, buf, dyn_size) == 0)
     {
       dyn_type *const dyn = (dyn_type *) buf;
 #if defined DT_MIPS_RLD_MAP
@@ -958,11 +957,11 @@ get_r_debug (netbsd_process_target *target, const int pid)
   return map;
 }

-/* Read one pointer from MEMADDR in the inferior.  */
+/* See netbsd-low.h.  */

-static int
-read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
-	      int ptr_size)
+int
+netbsd_process_target::read_one_ptr (CORE_ADDR memaddr, CORE_ADDR *ptr,
+				     int ptr_size)
 {
   /* Go through a union so this works on either big or little endian
      hosts, when the inferior's pointer size is smaller than the size
@@ -976,7 +975,7 @@ read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
     unsigned char uc;
   } addr;

-  int ret = target->read_memory (memaddr, &addr.uc, ptr_size);
+  int ret = read_memory (memaddr, &addr.uc, ptr_size);
   if (ret == 0)
     {
       if (ptr_size == sizeof (CORE_ADDR))
@@ -989,15 +988,39 @@ read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
   return ret;
 }

+/* See netbsd-low.h.  */
+
+bool
+netbsd_process_target::elf_64_file_p (const char *file)
+{
+  int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
+  if (fd < 0)
+    perror_with_name (("open"));
+
+  Elf64_Ehdr header;
+  ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::read, fd, &header, sizeof (header));
+  if (ret == -1)
+    perror_with_name (("read"));
+  gdb::handle_eintr<int> (-1, ::close, fd);
+  if (ret != sizeof (header))
+    error ("Cannot read ELF file header: %s", file);
+
+  if (header.e_ident[EI_MAG0] != ELFMAG0
+      || header.e_ident[EI_MAG1] != ELFMAG1
+      || header.e_ident[EI_MAG2] != ELFMAG2
+      || header.e_ident[EI_MAG3] != ELFMAG3)
+    error ("Unrecognized ELF file header: %s", file);
+
+  return header.e_ident[EI_CLASS] == ELFCLASS64;
+}
+
 /* Construct qXfer:libraries-svr4:read reply.  */

-template <typename T>
 int
-netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
-			     const pid_t pid, const char *annex,
-			     unsigned char *readbuf,
-			     unsigned const char *writebuf,
-			     CORE_ADDR offset, int len)
+netbsd_process_target::qxfer_libraries_svr4 (const char *annex,
+					     unsigned char *readbuf,
+					     unsigned const char *writebuf,
+					     CORE_ADDR offset, int len)
 {
   struct link_map_offsets
   {
@@ -1049,10 +1072,18 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
   CORE_ADDR l_name, l_addr, l_ld, l_next, l_prev;
   int header_done = 0;

+  if (writebuf != nullptr)
+    return -2;
+  if (readbuf == nullptr)
+    return -1;
+
+  struct process_info *proc = current_process ();
+  pid_t pid = proc->pid;
+  bool is_elf64 = elf_64_file_p (netbsd_nat::pid_to_exec_file (pid));
+
   const struct link_map_offsets *lmo
-    = ((sizeof (T) == sizeof (int64_t))
-       ? &lmo_64bit_offsets : &lmo_32bit_offsets);
-  int ptr_size = sizeof (T);
+    = (is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets);
+  int ptr_size = is_elf64 ? sizeof (int64_t) : sizeof (int32_t);

   while (annex[0] != '\0')
     {
@@ -1080,7 +1111,12 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,

   if (lm_addr == 0)
     {
-      CORE_ADDR r_debug = get_r_debug<T> (target, pid);
+      CORE_ADDR r_debug;
+
+      if (is_elf64)
+	r_debug = get_r_debug<int64_t> (pid);
+      else
+	r_debug = get_r_debug<int32_t> (pid);

       /* We failed to find DT_DEBUG.  Such situation will not change
 	 for this inferior - do not retry it.  Report it to GDB as
@@ -1091,7 +1127,7 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
       if (r_debug != 0)
 	{
 	  CORE_ADDR map_offset = r_debug + lmo->r_map_offset;
-	  if (read_one_ptr (target, map_offset, &lm_addr, ptr_size) != 0)
+	  if (read_one_ptr (map_offset, &lm_addr, ptr_size) != 0)
 	    warning ("unable to read r_map from %s",
 		     core_addr_to_string (map_offset));
 	}
@@ -1100,15 +1136,15 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
   std::string document = "<library-list-svr4 version=\"1.0\"";

   while (lm_addr
-	 && read_one_ptr (target, lm_addr + lmo->l_name_offset,
+	 && read_one_ptr (lm_addr + lmo->l_name_offset,
 			  &l_name, ptr_size) == 0
-	 && read_one_ptr (target, lm_addr + lmo->l_addr_offset,
+	 && read_one_ptr (lm_addr + lmo->l_addr_offset,
 			  &l_addr, ptr_size) == 0
-	 && read_one_ptr (target, lm_addr + lmo->l_ld_offset,
+	 && read_one_ptr (lm_addr + lmo->l_ld_offset,
 			  &l_ld, ptr_size) == 0
-	 && read_one_ptr (target, lm_addr + lmo->l_prev_offset,
+	 && read_one_ptr (lm_addr + lmo->l_prev_offset,
 			  &l_prev, ptr_size) == 0
-	 && read_one_ptr (target, lm_addr + lmo->l_next_offset,
+	 && read_one_ptr (lm_addr + lmo->l_next_offset,
 			  &l_next, ptr_size) == 0)
     {
       if (lm_prev != l_prev)
@@ -1134,7 +1170,7 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
 	  /* Not checking for error because reading may stop before
 	     we've got PATH_MAX worth of characters.  */
 	  libname[0] = '\0';
-	  target->read_memory (l_name, libname, sizeof (libname) - 1);
+	  read_memory (l_name, libname, sizeof (libname) - 1);
 	  libname[sizeof (libname) - 1] = '\0';
 	  if (libname[0] != '\0')
 	    {
@@ -1179,57 +1215,6 @@ netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
   return len;
 }

-/* See netbsd-low.h.  */
-
-bool
-netbsd_process_target::elf_64_file_p (const char *file)
-{
-  int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
-  if (fd < 0)
-    perror_with_name (("open"));
-
-  Elf64_Ehdr header;
-  ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::read, fd, &header, sizeof (header));
-  if (ret == -1)
-    perror_with_name (("read"));
-  gdb::handle_eintr<int> (-1, ::close, fd);
-  if (ret != sizeof (header))
-    error ("Cannot read ELF file header: %s", file);
-
-  if (header.e_ident[EI_MAG0] != ELFMAG0
-      || header.e_ident[EI_MAG1] != ELFMAG1
-      || header.e_ident[EI_MAG2] != ELFMAG2
-      || header.e_ident[EI_MAG3] != ELFMAG3)
-    error ("Unrecognized ELF file header: %s", file);
-
-  return header.e_ident[EI_CLASS] == ELFCLASS64;
-}
-
-/* Construct qXfer:libraries-svr4:read reply.  */
-
-int
-netbsd_process_target::qxfer_libraries_svr4 (const char *annex,
-					     unsigned char *readbuf,
-					     unsigned const char *writebuf,
-					     CORE_ADDR offset, int len)
-{
-  if (writebuf != nullptr)
-    return -2;
-  if (readbuf == nullptr)
-    return -1;
-
-  struct process_info *proc = current_process ();
-  pid_t pid = proc->pid;
-  bool is_elf64 = elf_64_file_p (netbsd_nat::pid_to_exec_file (pid));
-
-  if (is_elf64)
-    return netbsd_qxfer_libraries_svr4<int64_t> (this, pid, annex, readbuf,
-						 writebuf, offset, len);
-  else
-    return netbsd_qxfer_libraries_svr4<int32_t> (this, pid, annex, readbuf,
-						 writebuf, offset, len);
-}
-
 /* Implement the supports_qxfer_libraries_svr4 target_ops method.  */

 bool
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index 7ed08dc90fa..338293d74a6 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -156,6 +156,26 @@ class netbsd_process_target : public process_stratum_target
   /* Returns true if GDB is interested in the reported SYSNO syscall.  */
   bool netbsd_catch_this_syscall (int sysno);

+  /* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
+  template <typename T> int
+  get_phdr_phnum_from_proc_auxv (const pid_t pid,
+				 CORE_ADDR *phdr_memaddr,
+				 int *num_phdr);
+
+  /* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
+  template <typename T> CORE_ADDR
+  get_dynamic (const pid_t pid);
+
+  /* Return &_r_debug in the inferior, or -1 if not present.  Return value
+     can be 0 if the inferior does not yet have the library list initialized.
+     We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
+     DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
+  template <typename T>
+  CORE_ADDR get_r_debug (const int pid);
+
+  /* Read one pointer from MEMADDR in the inferior.  */
+  int read_one_ptr (CORE_ADDR memaddr, CORE_ADDR *ptr, int ptr_size);
+
 protected:
   /* The architecture-specific "low" methods are listed below.  */

--
2.28.0

[PATCH 10/10] Fix whitespace formatting

[Kamil Rytarowski]

gdbserver/ChangeLog:

        * netbsd-low.cc: Fix whitespace formatting.
        * netbsd-amd64-low.cc: Likewise.
---
 gdbserver/ChangeLog           |  5 +++++
 gdbserver/netbsd-amd64-low.cc | 10 +++++-----
 gdbserver/netbsd-low.cc       |  4 ++--
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 0c1c40e0581..0c29ae136d4 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2020-10-01  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc: Fix whitespace formatting.
+	* netbsd-amd64-low.cc: Likewise.
+
 2020-10-01  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-low.cc (get_phdr_phnum_from_proc_auxv): Turn into...
diff --git a/gdbserver/netbsd-amd64-low.cc b/gdbserver/netbsd-amd64-low.cc
index c59ebc8f2cf..f3d30d7e7cc 100644
--- a/gdbserver/netbsd-amd64-low.cc
+++ b/gdbserver/netbsd-amd64-low.cc
@@ -159,11 +159,11 @@ netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)

 static const struct netbsd_regset_info netbsd_target_regsets[] =
 {
- /* General Purpose Registers.  */
- {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
-  netbsd_x86_64_fill_gregset, netbsd_x86_64_store_gregset},
- /* End of list marker.  */
- {0, 0, -1, NULL, NULL }
+  /* General Purpose Registers.  */
+  {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
+   netbsd_x86_64_fill_gregset, netbsd_x86_64_store_gregset},
+  /* End of list marker.  */
+  {0, 0, -1, NULL, NULL }
 };

 /* NetBSD target op definitions for the amd64 architecture.  */
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index f1011cdfe73..cde4c5682b4 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -615,7 +615,7 @@ netbsd_process_target::request_interrupt ()
 {
   ptid_t inferior_ptid = ptid_of (get_first_thread ());

-  ::kill (inferior_ptid.pid(), SIGINT);
+  ::kill (inferior_ptid.pid (), SIGINT);
 }

 /* Read the AUX Vector for the specified PID, wrapping the ptrace(2) call
@@ -876,7 +876,7 @@ netbsd_process_target::get_dynamic (const pid_t pid)
   CORE_ADDR relocation = -1;
   for (int i = 0; relocation == -1 && i < num_phdr; i++)
     {
-      phdr_type *const p = (phdr_type *) (phdr_buf.data() + i * phdr_size);
+      phdr_type *const p = (phdr_type *) (phdr_buf.data () + i * phdr_size);

       if (p->p_type == PT_PHDR)
 	relocation = phdr_memaddr - p->p_vaddr;
--
2.28.0

Re: [PATCH 01/10] Merge netbsd_ptrace_fun into the netbsd_process_target class

[Simon Marchi]

On 2020-10-01 10:17 p.m., Kamil Rytarowski wrote:
> Change netbsd_ptrace_fun into a local lambda function, as otherwise
> if it is a class member it is harder to pass it as a callbakc function

callbakc -> callback

I'm not sure I see the advantage of having it as a lambda over a free
function as it is now (not that I see a disadvantage either, and maybe
that will become evident in the following patches), but if it makes you
happy, this is ok.

> @@ -96,8 +64,39 @@ netbsd_process_target::create_inferior (const char *program,
>  {
>    std::string str_program_args = construct_inferior_arguments (program_args);
>
> +  /* Callback used by fork_inferior to start tracing the inferior.  */
> +  auto fn

I'd suggest choosing a more meaningful name than "fn", maybe
"traceme_fn", to match the fork_inferior parameter name.

Simon

Re: [PATCH 02/10] Include elf_64_file_p in the netbsd_process_target class

[Simon Marchi]

On 2020-10-01 10:17 p.m., Kamil Rytarowski wrote:
> gdbserver/ChangeLog:
>
>         * netbsd-low.cc (elf_64_file_p): Turn into...
>         (netbsd_process_target::elf_64_file_p): ...this.
>         * netbsd-low.h (netbsd_process_target::elf_64_file_p): Add.

I don't see the point in making this a member function.  It doesn't
interact with netbsd_process_target class in any way, so low cohesion.
So the net result is that if you need to use this quite generic function
from outside the netbsd_process_target in the future, you can't.

I'm not strongly opposing, since this is not a particularly important
matter, I just find it odd.

Simon

Re: [PATCH 03/10] Remove unneeded netbsd_add_process()

[Simon Marchi]

On 2020-10-01 10:17 p.m., Kamil Rytarowski wrote:
> Currently it does not add any value.
> 
> The netbsd_tdesc local variable is no longer needed. Remove it.
> The tdesc value is set by the low target now.

Thanks, this is ok.

Simon

Re: [PATCH 04/10] Include gdb_catching_syscalls_p in the netbsd_process_target class

[Simon Marchi]

On 2020-10-01 10:17 p.m., Kamil Rytarowski wrote:
> gdb/ChangeLog:
>
>         * netbsd-low.cc (netbsd_tdesc): Remove.
>         (netbsd_add_process): Likewise.
>         (netbsd_process_target::create_inferior): Update.

Same comment as the previous patch.

Simon

Re: [PATCH 05/10] Include netbsd_wait in the netbsd_process_target class

[Simon Marchi]

On 2020-10-01 10:17 p.m., Kamil Rytarowski wrote:
> gdbserver/ChangeLog:
>
>        * netbsd-low.cc (netbsd_wait): Turn into...
>        (netbsd_process_target::netbsd_wait): ...this.
>        * netbsd-low.h (netbsd_process_target::netbsd_wait): Add.

For patches 5, 6 and 7, I can kind of see the point.  These are all
helpers for netbsd_process_target::wait (instead of writing a gigantic
method), so I can get that it makes sense for cohesion to keep them as
member functions.

I can only see a downside to doing this though: if one of these becomes
unused, the compiler won't tell you, unlike with a static function.

Simon

Re: [PATCH 08/10] Include netbsd_catch_this_syscall in the netbsd_process_target class

[Simon Marchi]

On 2020-10-01 10:18 p.m., Kamil Rytarowski wrote:
> gdbserver/ChangeLog:
>
>        * netbsd-low.cc (netbsd_catch_this_syscall): Turn into...
>        (netbsd_process_target::netbsd_catch_this_syscall): ...this.
>        * netbsd-low.h (netbsd_process_target::netbsd_catch_this_syscall): Add.

Same comment as before.

Simon

Re: [PATCH 09/10] Include the functions of qxfer_libraries_svr4 in netbsd_process_target

[Simon Marchi]

On 2020-10-01 10:18 p.m., Kamil Rytarowski wrote:
> @@ -848,11 +850,11 @@ int get_phdr_phnum_from_proc_auxv (const pid_t pid,
>    return 0;
>  }
>
> -/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
> +/* See netbsd-low.h.  */
>
>  template <typename T>
> -static CORE_ADDR
> -get_dynamic (netbsd_process_target *target, const pid_t pid)
> +CORE_ADDR
> +netbsd_process_target::get_dynamic (const pid_t pid)

I think this change here makes sense.  Instead of passing the target as
a parameter, it makes sense to have this as a method.

For the other functions that don't care about the target object, same
comment as before.

Simon

Re: [PATCH 10/10] Fix whitespace formatting

[Simon Marchi]

On 2020-10-01 10:18 p.m., Kamil Rytarowski wrote:
> gdbserver/ChangeLog:
> 
>         * netbsd-low.cc: Fix whitespace formatting.
>         * netbsd-amd64-low.cc: Likewise.

Thanks, feel free to push these as obvious.

Simon

Re: [PATCH 01/10] Merge netbsd_ptrace_fun into the netbsd_process_target class

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1261 bytes --]

On 07.10.2020 04:37, Simon Marchi wrote:
> On 2020-10-01 10:17 p.m., Kamil Rytarowski wrote:
>> Change netbsd_ptrace_fun into a local lambda function, as otherwise
>> if it is a class member it is harder to pass it as a callbakc function
> 
> callbakc -> callback
> 
> I'm not sure I see the advantage of having it as a lambda over a free
> function as it is now (not that I see a disadvantage either, and maybe
> that will become evident in the following patches), but if it makes you
> happy, this is ok.
> 

I prefer to keep all the local static functions in gdbserver citizens of
the NetBSD gdbserver. Long term, shared functionality with the native
target will go to gdb/nat and perhaps some functions like
elf_64_file_p() could be shared by ELF targets.

This refers to all the follow up patches.

>> @@ -96,8 +64,39 @@ netbsd_process_target::create_inferior (const char *program,
>>  {
>>    std::string str_program_args = construct_inferior_arguments (program_args);
>>
>> +  /* Callback used by fork_inferior to start tracing the inferior.  */
>> +  auto fn
> 
> I'd suggest choosing a more meaningful name than "fn", maybe
> "traceme_fn", to match the fork_inferior parameter name.
> 

I will do it.

> Simon
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 01/10] Merge netbsd_ptrace_fun into the netbsd_process_target class

[Simon Marchi]

On 2020-10-07 12:36 a.m., Kamil Rytarowski wrote:
> I prefer to keep all the local static functions in gdbserver citizens of
> the NetBSD gdbserver.

I don't understand what this means.

> Long term, shared functionality with the native
> target will go to gdb/nat and perhaps some functions like
> elf_64_file_p() could be shared by ELF targets.

If the goal is sharing them in gdb/nat, it seems to me like it's easier
to keep it as a free function, that will be easier to share.

Simon

Re: [PATCH 01/10] Merge netbsd_ptrace_fun into the netbsd_process_target class

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 772 bytes --]

On 07.10.2020 13:19, Simon Marchi wrote:
> On 2020-10-07 12:36 a.m., Kamil Rytarowski wrote:
>> I prefer to keep all the local static functions in gdbserver citizens of
>> the NetBSD gdbserver.
> 
> I don't understand what this means.
> 

I mean I prefer having functions as members of the class.

>> Long term, shared functionality with the native
>> target will go to gdb/nat and perhaps some functions like
>> elf_64_file_p() could be shared by ELF targets.
> 
> If the goal is sharing them in gdb/nat, it seems to me like it's easier
> to keep it as a free function, that will be easier to share.
> 


OK. I will abandon the idea of adding the freestanding functions to the
target class, unless they interact the class members.

> Simon
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]