[rfc][3/4][rft/procfs] Move "info proc" command to common code

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Hello,

this patch removes the somewhat odd situation that procfs.c and linux-nat.c
each install their own "info proc" command.  Instead, the command and its
subcommands are now installed in infcmd.c common code, which also performs
the basic argument parsing.

Actual output of /proc information is then triggered via new target operation
(to_info_proc).  The existing code in procfs.c and linux-nat.c is repurposed
as implementation of that target op.

Overall, there ought to be no significant change in behaviour due to this
patch, except as brought in due to the common argument parsing:

- procfs.c gains the explicitly enumerated subcommands (even though it will
  not actually execute any other than "mappings" and "all").

- linux-nat.c gains parsing of thread IDs ("PID/TID" format), even though it
  will ignore the TID since threads share the same /proc information

- there may be some change of behaviour with erroneous input

In particular, as this stage we continue to support info proc with a PID
argument refering to other processes.

Bye,
Ulrich



ChangeLog:

	* defs.h (enum info_proc_what): Moved here from linux-nat.c
	* infcmd.c: Include "cli/cli-utils.h".
	(info_proc_cmd_1): New function.
	(info_proc_cmd): New function, moved here from equivalent routine
	orignally in linux-nat.c.
	(info_proc_cmd_mappings): Likewise.
	(info_proc_cmd_stat): Likewise.
	(info_proc_cmd_status): Likewise.
	(info_proc_cmd_cwd): Likewise.
	(info_proc_cmd_cmdline): Likewise.
	(info_proc_cmd_exe): Likewise.
	(info_proc_cmd_all): Likewise.
	(_initialize_infcmd): Install "info proc" command and subcommands.

	* target.h (struct target_ops): Add to_info_proc.
	(target_info_proc): Add prototype.
	* target.c (target_info_proc): New function.

	* procfs.c (procfs_info_proc): Add prototype.
	(info_proc_cmd): Rename into ...
	(procfs_info_proc): ... this.  Update argument types as appropriate
	for a to_info_proc implementation.  Remove argument parsing.
	(procfs_target): Install procfs_info_proc.
	(_initialize_procfs): No longer install "info proc" command.

	* linux-nat.c: Do not include "cli/cli-utils.h".
	(enum info_proc_what): Remove.
	(linux_nat_info_proc_cmd_1): Rename into ...
	(linux_nat_info_proc): ... this.  Update argument types as appropriate
	for a to_info_proc implementation.  Remove argument parsing.
	(linux_nat_info_proc_cmd): Remove.
	(linux_nat_info_proc_cmd_mappings): Likewise.
	(linux_nat_info_proc_cmd_stat): Likewise.
	(linux_nat_info_proc_cmd_status): Likewise.
	(linux_nat_info_proc_cmd_cwd): Likewise.
	(linux_nat_info_proc_cmd_cmdline): Likewise.
	(linux_nat_info_proc_cmd_exe): Likewise.
	(linux_nat_info_proc_cmd_all): Likewise.
	(linux_target_install_ops): Install linux_nat_info_proc.
	(_initialize_linux_nat): No longer install "info proc" command
	and subcommands.

testsuite/ChangeLog:

	* gdb.base/info-proc.exp: Also run on remote targets.  Main
	"info proc" command is now always present; whether target supports
	actual info proc operation is detected when attempting to issue
	the command.


Index: gdb-head/gdb/target.h
===================================================================
--- gdb-head.orig/gdb/target.h
+++ gdb-head/gdb/target.h
@@ -683,6 +683,9 @@ struct target_ops
     struct address_space *(*to_thread_address_space) (struct target_ops *,
 						      ptid_t);
 
+    /* Show information about process PTID.  */
+    void (*to_info_proc) (struct target_ops *, ptid_t, enum info_proc_what);
+
     /* Tracepoint-related operations.  */
 
     /* Prepare the target for a tracing run.  */
@@ -910,6 +913,10 @@ extern void target_store_registers (stru
 
 struct address_space *target_thread_address_space (ptid_t);
 
+/* Show information about process PTID.  */
+
+void target_info_proc (ptid_t, enum info_proc_what);
+
 /* Returns true if this target can debug multiple processes
    simultaneously.  */
 
Index: gdb-head/gdb/procfs.c
===================================================================
--- gdb-head.orig/gdb/procfs.c
+++ gdb-head/gdb/procfs.c
@@ -152,6 +152,9 @@ static char * procfs_make_note_section (
 
 static int procfs_can_use_hw_breakpoint (int, int, int);
 
+static void procfs_info_proc (struct target_ops *, ptid_t,
+			      enum info_proc_what);
+
 #if defined (PR_MODEL_NATIVE) && (PR_MODEL_NATIVE == PR_MODEL_LP64)
 /* When GDB is built as 64-bit application on Solaris, the auxv data
    is presented in 64-bit format.  We need to provide a custom parser
@@ -212,6 +215,7 @@ procfs_target (void)
   t->to_has_thread_control = tc_schedlock;
   t->to_find_memory_regions = proc_find_memory_regions;
   t->to_make_corefile_notes = procfs_make_note_section;
+  t->to_info_proc = procfs_info_proc;
 
 #if defined(PR_MODEL_NATIVE) && (PR_MODEL_NATIVE == PR_MODEL_LP64)
   t->to_auxv_parse = procfs_auxv_parse;
@@ -5391,63 +5395,42 @@ info_proc_mappings (procinfo *pi, int su
 /* Implement the "info proc" command.  */
 
 static void
-info_proc_cmd (char *args, int from_tty)
+procfs_info_proc (struct target_ops *ops, ptid_t ptid,
+		  enum info_proc_what what)
 {
   struct cleanup *old_chain;
   procinfo *process  = NULL;
   procinfo *thread   = NULL;
-  char    **argv     = NULL;
-  char     *tmp      = NULL;
-  int       pid      = 0;
-  int       tid      = 0;
+  int       pid      = ptid_get_pid (ptid);
+  int       tid      = ptid_get_tid (ptid);
   int       mappings = 0;
 
-  old_chain = make_cleanup (null_cleanup, 0);
-  if (args)
-    {
-      argv = gdb_buildargv (args);
-      make_cleanup_freeargv (argv);
-    }
-  while (argv != NULL && *argv != NULL)
+  switch (what)
     {
-      if (isdigit (argv[0][0]))
-	{
-	  pid = strtoul (argv[0], &tmp, 10);
-	  if (*tmp == '/')
-	    tid = strtoul (++tmp, NULL, 10);
-	}
-      else if (argv[0][0] == '/')
-	{
-	  tid = strtoul (argv[0] + 1, NULL, 10);
-	}
-      else if (strncmp (argv[0], "mappings", strlen (argv[0])) == 0)
-	{
-	  mappings = 1;
-	}
-      else
-	{
-	  /* [...] */
-	}
-      argv++;
+    case IP_MINIMAL:
+      break;
+
+    case IP_MAPPINGS:
+    case IP_ALL:
+      mappings = 1;
+      break;
+
+    default:
+      error (_("Not supported on this target."));
     }
-  if (pid == 0)
-    pid = PIDGET (inferior_ptid);
-  if (pid == 0)
-    error (_("No current process: you must name one."));
-  else
+
+  old_chain = make_cleanup (null_cleanup, 0);
+
+  /* First see if it's a process we're already debugging.  */
+  process = find_procinfo (pid, 0);
+  if (process == NULL)
     {
-      /* Have pid, will travel.
-	 First see if it's a process we're already debugging.  */
-      process = find_procinfo (pid, 0);
-       if (process == NULL)
-	 {
-	   /* No.  So open a procinfo for it, but
-	      remember to close it again when finished.  */
-	   process = create_procinfo (pid, 0);
-	   make_cleanup (do_destroy_procinfo_cleanup, process);
-	   if (!open_procinfo_files (process, FD_CTL))
-	     proc_error (process, "info proc, open_procinfo_files", __LINE__);
-	 }
+      /* No.  So open a procinfo for it, but
+	 remember to close it again when finished.  */
+      process = create_procinfo (pid, 0);
+      make_cleanup (do_destroy_procinfo_cleanup, process);
+      if (!open_procinfo_files (process, FD_CTL))
+	proc_error (process, "info proc, open_procinfo_files", __LINE__);
     }
   if (tid != 0)
     thread = create_procinfo (pid, tid);
@@ -5568,10 +5551,6 @@ _initialize_procfs (void)
 {
   observer_attach_inferior_created (procfs_inferior_created);
 
-  add_info ("proc", info_proc_cmd, _("\
-Show /proc process information about any running process.\n\
-Specify process id, or use the program being debugged by default.\n\
-Specify keyword 'mappings' for detailed info on memory mappings."));
   add_com ("proc-trace-entry", no_class, proc_trace_sysentry_cmd,
 	   _("Give a trace of entries into the syscall."));
   add_com ("proc-trace-exit", no_class, proc_trace_sysexit_cmd,
Index: gdb-head/gdb/target.c
===================================================================
--- gdb-head.orig/gdb/target.c
+++ gdb-head/gdb/target.c
@@ -3112,6 +3112,31 @@ target_thread_address_space (ptid_t ptid
   return inf->aspace;
 }
 
+/* Show information about process PTID.  */
+
+void
+target_info_proc (ptid_t ptid, enum info_proc_what what)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_info_proc != NULL)
+	{
+	  t->to_info_proc (t, ptid, what);
+
+	  if (targetdebug)
+	    fprintf_unfiltered (gdb_stdlog,
+				"target_info_proc (%s, %d)\n",
+				target_pid_to_str (ptid), what);
+
+	  return;
+	}
+    }
+
+  error (_("Not supported on this target."));
+}
+
 static int
 default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 {
Index: gdb-head/gdb/linux-nat.c
===================================================================
--- gdb-head.orig/gdb/linux-nat.c
+++ gdb-head/gdb/linux-nat.c
@@ -60,7 +60,6 @@
 #include "solib.h"
 #include "linux-osdata.h"
 #include "linux-procfs.h"
-#include "cli/cli-utils.h"
 
 #ifndef SPUFS_MAGIC
 #define SPUFS_MAGIC 0x23c9b64e
@@ -4749,39 +4748,11 @@ linux_nat_make_corefile_notes (bfd *obfd
 
 /* Implement the "info proc" command.  */
 
-enum info_proc_what
-  {
-    /* Display the default cmdline, cwd and exe outputs.  */
-    IP_MINIMAL,
-
-    /* Display `info proc mappings'.  */
-    IP_MAPPINGS,
-
-    /* Display `info proc status'.  */
-    IP_STATUS,
-
-    /* Display `info proc stat'.  */
-    IP_STAT,
-
-    /* Display `info proc cmdline'.  */
-    IP_CMDLINE,
-
-    /* Display `info proc exe'.  */
-    IP_EXE,
-
-    /* Display `info proc cwd'.  */
-    IP_CWD,
-
-    /* Display all of the above.  */
-    IP_ALL
-  };
-
 static void
-linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
+linux_nat_info_proc (struct target_ops *ops, ptid_t ptid,
+		     enum info_proc_what what)
 {
-  /* A long is used for pid instead of an int to avoid a loss of precision
-     compiler warning from the output of strtoul.  */
-  long pid = PIDGET (inferior_ptid);
+  long pid = ptid_get_pid (ptid);
   FILE *procfile;
   char buffer[MAXPATHLEN];
   char fname1[MAXPATHLEN], fname2[MAXPATHLEN];
@@ -4793,16 +4764,6 @@ linux_nat_info_proc_cmd_1 (char *args, e
   int stat_f = (what == IP_STAT || what == IP_ALL);
   struct stat dummy;
 
-  if (args && isdigit (args[0]))
-    pid = strtoul (args, &args, 10);
-
-  args = skip_spaces (args);
-  if (args && args[0])
-    error (_("Too many parameters: %s"), args);
-
-  if (pid == 0)
-    error (_("No current process: you must name one."));
-
   sprintf (fname1, "/proc/%ld", pid);
   if (stat (fname1, &dummy) != 0)
     error (_("No /proc directory: '%s'"), fname1);
@@ -5018,70 +4979,6 @@ linux_nat_info_proc_cmd_1 (char *args, e
     }
 }
 
-/* Implement `info proc' when given without any futher parameters.  */
-
-static void
-linux_nat_info_proc_cmd (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_MINIMAL, from_tty);
-}
-
-/* Implement `info proc mappings'.  */
-
-static void
-linux_nat_info_proc_cmd_mappings (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_MAPPINGS, from_tty);
-}
-
-/* Implement `info proc stat'.  */
-
-static void
-linux_nat_info_proc_cmd_stat (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_STAT, from_tty);
-}
-
-/* Implement `info proc status'.  */
-
-static void
-linux_nat_info_proc_cmd_status (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_STATUS, from_tty);
-}
-
-/* Implement `info proc cwd'.  */
-
-static void
-linux_nat_info_proc_cmd_cwd (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_CWD, from_tty);
-}
-
-/* Implement `info proc cmdline'.  */
-
-static void
-linux_nat_info_proc_cmd_cmdline (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_CMDLINE, from_tty);
-}
-
-/* Implement `info proc exe'.  */
-
-static void
-linux_nat_info_proc_cmd_exe (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_EXE, from_tty);
-}
-
-/* Implement `info proc all'.  */
-
-static void
-linux_nat_info_proc_cmd_all (char *args, int from_tty)
-{
-  linux_nat_info_proc_cmd_1 (args, IP_ALL, from_tty);
-}
-
 /* Implement the to_xfer_partial interface for memory reads using the /proc
    filesystem.  Because we can use a single read() call for /proc, this
    can be much more efficient than banging away at PTRACE_PEEKTEXT,
@@ -5315,6 +5212,7 @@ linux_target_install_ops (struct target_
   t->to_follow_fork = linux_child_follow_fork;
   t->to_find_memory_regions = linux_nat_find_memory_regions;
   t->to_make_corefile_notes = linux_nat_make_corefile_notes;
+  t->to_info_proc = linux_nat_info_proc;
 
   super_xfer_partial = t->to_xfer_partial;
   t->to_xfer_partial = linux_xfer_partial;
@@ -5814,43 +5712,6 @@ extern initialize_file_ftype _initialize
 void
 _initialize_linux_nat (void)
 {
-  static struct cmd_list_element *info_proc_cmdlist;
-
-  add_prefix_cmd ("proc", class_info, linux_nat_info_proc_cmd,
-		  _("\
-Show /proc process information about any running process.\n\
-Specify any process id, or use the program being debugged by default."),
-		  &info_proc_cmdlist, "info proc ",
-		  1/*allow-unknown*/, &infolist);
-
-  add_cmd ("mappings", class_info, linux_nat_info_proc_cmd_mappings, _("\
-List of mapped memory regions."),
-	   &info_proc_cmdlist);
-
-  add_cmd ("stat", class_info, linux_nat_info_proc_cmd_stat, _("\
-List process info from /proc/PID/stat."),
-	   &info_proc_cmdlist);
-
-  add_cmd ("status", class_info, linux_nat_info_proc_cmd_status, _("\
-List process info from /proc/PID/status."),
-	   &info_proc_cmdlist);
-
-  add_cmd ("cwd", class_info, linux_nat_info_proc_cmd_cwd, _("\
-List current working directory of the process."),
-	   &info_proc_cmdlist);
-
-  add_cmd ("cmdline", class_info, linux_nat_info_proc_cmd_cmdline, _("\
-List command line arguments of the process."),
-	   &info_proc_cmdlist);
-
-  add_cmd ("exe", class_info, linux_nat_info_proc_cmd_exe, _("\
-List absolute filename for executable of the process."),
-	   &info_proc_cmdlist);
-
-  add_cmd ("all", class_info, linux_nat_info_proc_cmd_all, _("\
-List all available /proc info."),
-	   &info_proc_cmdlist);
-
   add_setshow_zinteger_cmd ("lin-lwp", class_maintenance,
 			    &debug_linux_nat, _("\
 Set debugging of GNU/Linux lwp module."), _("\
Index: gdb-head/gdb/infcmd.c
===================================================================
--- gdb-head.orig/gdb/infcmd.c
+++ gdb-head/gdb/infcmd.c
@@ -57,6 +57,7 @@
 #include "tracepoint.h"
 #include "inf-loop.h"
 #include "continuations.h"
+#include "cli/cli-utils.h"
 
 /* Functions exported for general use, in inferior.h: */
 
@@ -2781,9 +2782,97 @@ unset_command (char *args, int from_tty)
   help_list (unsetlist, "unset ", -1, gdb_stdout);
 }
 
+/* Implement `info proc' family of commands.  */
+
+static void
+info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
+{
+  long pid = PIDGET (inferior_ptid);
+  long tid = 0;
+
+  if (args && isdigit (args[0]))
+    pid = strtoul (args, &args, 10);
+  if (args && *args == '/')
+    tid = strtoul (args + 1, &args, 10);
+
+  args = skip_spaces (args);
+  if (args && args[0])
+    error (_("Too many parameters: %s"), args);
+
+  if (pid == 0)
+    error (_("No current process: you must name one."));
+
+  target_info_proc (ptid_build (pid, 0, tid), what);
+}
+
+/* Implement `info proc' when given without any futher parameters.  */
+
+static void
+info_proc_cmd (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_MINIMAL, from_tty);
+}
+
+/* Implement `info proc mappings'.  */
+
+static void
+info_proc_cmd_mappings (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_MAPPINGS, from_tty);
+}
+
+/* Implement `info proc stat'.  */
+
+static void
+info_proc_cmd_stat (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_STAT, from_tty);
+}
+
+/* Implement `info proc status'.  */
+
+static void
+info_proc_cmd_status (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_STATUS, from_tty);
+}
+
+/* Implement `info proc cwd'.  */
+
+static void
+info_proc_cmd_cwd (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_CWD, from_tty);
+}
+
+/* Implement `info proc cmdline'.  */
+
+static void
+info_proc_cmd_cmdline (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_CMDLINE, from_tty);
+}
+
+/* Implement `info proc exe'.  */
+
+static void
+info_proc_cmd_exe (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_EXE, from_tty);
+}
+
+/* Implement `info proc all'.  */
+
+static void
+info_proc_cmd_all (char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_ALL, from_tty);
+}
+
 void
 _initialize_infcmd (void)
 {
+  static struct cmd_list_element *info_proc_cmdlist;
   struct cmd_list_element *c = NULL;
 
   /* Add the filename of the terminal connected to inferior I/O.  */
@@ -3010,4 +3099,39 @@ Register name as argument means describe
 
   add_info ("vector", vector_info,
 	    _("Print the status of the vector unit\n"));
+
+  add_prefix_cmd ("proc", class_info, info_proc_cmd,
+		  _("\
+Show /proc process information about any running process.\n\
+Specify any process id, or use the program being debugged by default."),
+		  &info_proc_cmdlist, "info proc ",
+		  1/*allow-unknown*/, &infolist);
+
+  add_cmd ("mappings", class_info, info_proc_cmd_mappings, _("\
+List of mapped memory regions."),
+	   &info_proc_cmdlist);
+
+  add_cmd ("stat", class_info, info_proc_cmd_stat, _("\
+List process info from /proc/PID/stat."),
+	   &info_proc_cmdlist);
+
+  add_cmd ("status", class_info, info_proc_cmd_status, _("\
+List process info from /proc/PID/status."),
+	   &info_proc_cmdlist);
+
+  add_cmd ("cwd", class_info, info_proc_cmd_cwd, _("\
+List current working directory of the process."),
+	   &info_proc_cmdlist);
+
+  add_cmd ("cmdline", class_info, info_proc_cmd_cmdline, _("\
+List command line arguments of the process."),
+	   &info_proc_cmdlist);
+
+  add_cmd ("exe", class_info, info_proc_cmd_exe, _("\
+List absolute filename for executable of the process."),
+	   &info_proc_cmdlist);
+
+  add_cmd ("all", class_info, info_proc_cmd_all, _("\
+List all available /proc info."),
+	   &info_proc_cmdlist);
 }
Index: gdb-head/gdb/defs.h
===================================================================
--- gdb-head.orig/gdb/defs.h
+++ gdb-head/gdb/defs.h
@@ -748,6 +748,35 @@ extern struct command_line *read_command
 
 extern void free_command_lines (struct command_line **);
 
+/* Parameters of the "info proc" command.  */
+
+enum info_proc_what
+  {
+    /* Display the default cmdline, cwd and exe outputs.  */
+    IP_MINIMAL,
+
+    /* Display `info proc mappings'.  */
+    IP_MAPPINGS,
+
+    /* Display `info proc status'.  */
+    IP_STATUS,
+
+    /* Display `info proc stat'.  */
+    IP_STAT,
+
+    /* Display `info proc cmdline'.  */
+    IP_CMDLINE,
+
+    /* Display `info proc exe'.  */
+    IP_EXE,
+
+    /* Display `info proc cwd'.  */
+    IP_CWD,
+
+    /* Display all of the above.  */
+    IP_ALL
+  };
+
 /* String containing the current directory (what getwd would return).  */
 
 extern char *current_directory;
Index: gdb-head/gdb/testsuite/gdb.base/info-proc.exp
===================================================================
--- gdb-head.orig/gdb/testsuite/gdb.base/info-proc.exp
+++ gdb-head/gdb/testsuite/gdb.base/info-proc.exp
@@ -21,11 +21,6 @@ if $tracelevel then {
 	strace $tracelevel
 }
 
-if { [is_remote target] } then {
-  continue
-}
-
-
 set ws "\[ \t\]+"
 
 set testfile "break"
@@ -55,17 +50,7 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-# Does this gdb support info proc?
-gdb_test_multiple "help info proc" "help info proc" {
-    -re "Undefined info command: .proc..  Try .help info.*$gdb_prompt $" {
-	# info proc command not supported -- nothing to test here.
-	unsupported "gdb does not support info proc on this target"
-	return -1;
-    }
-    -re "Show /proc process information about .*$gdb_prompt $" {
-	pass "help info proc"
-    }
-}
+gdb_test "help info proc" "Show /proc process information about .*"
 
 gdb_test "info proc" "No current process.*" "info proc without a process"
 
@@ -74,7 +59,17 @@ if { ! [ runto_main ] } then {
     return -1
 }
 
-gdb_test "info proc" "process ${decimal}.*" "info proc with process"
+# Does this gdb support info proc?
+gdb_test_multiple "info proc" "info proc" {
+    -re "Not supported on this target.*$gdb_prompt $" {
+	# info proc command not supported -- nothing to test here.
+	unsupported "gdb does not support info proc on this target"
+	return -1;
+    }
+    -re "process ${decimal}.*$gdb_prompt $" {
+	pass "info proc with process"
+    }
+}
 
 gdb_test "info proc mapping" \
 	".*Mapped address spaces:.*${hex}${ws}${hex}${ws}${hex}${ws}${hex}.*" \
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com