Re: [patch] PID cmdline argument should be whole from digits

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Sat, 13 Feb 2010 18:44:54 +0100, Eli Zaretskii wrote:
> > Date: Sat, 13 Feb 2010 16:20:44 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > 
> > +      if (pid_or_core_arg[0] != 0
> > +	  && strspn (pid_or_core_arg, "0123456789") == strlen (pid_or_core_arg))
> 
> Should we disallow PIDs in hex?

Thanks for the notice, the patch was probably more wrong as the PID
interpretation should be more left on the target's interpretation.

Attached patch no longer tries to attach although it will print there:
	Illegal process-id: 1.core.

No problem dropping the patch, I thought it will be easy, it is not.

Verified Microsoft Visual Studio 2010 (beta?) supports neither of getpid,
__getpid nor pid_t.


No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


gdb/
2010-02-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* configure.ac (AC_CHECK_FUNCS): Check for getpid.
	* configure: Regenerate.
	* config.in: Regenerate.
	* defs.h (parse_pid): New.
	* utils.c (parse_pid): New.
	* darwin-nat.c (darwin_attach): Replace ARGS parsing by parse_pid.
	* gnu-nat.c (gnu_attach): Likewise.
	* nto-procfs.c (procfs_attach): Likewise.
	* procfs.c (procfs_attach): Likewise.
	* windows-nat.c (windows_attach): Likewise.
	* inf-ptrace.c (inf_ptrace_attach): Likewise.  Remove variable dummy.
	* inf-ttrace.c (inf_ttrace_attach): Likewise.
	* remote.c (extended_remote_attach_1): Likewise.

gdb/testsuite/
2010-02-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/default.exp (attach): Update the expect string.

--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -799,7 +799,7 @@ AC_FUNC_ALLOCA
 AC_FUNC_MMAP
 AC_FUNC_VFORK
 AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
-		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
+		getpid getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
 		ttrace wborder setlocale iconvlist libiconvlist btowc \
 		setrlimit getrlimit])
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1514,13 +1514,7 @@ darwin_attach (struct target_ops *ops, char *args, int from_tty)
   struct inferior *inf;
   kern_return_t kret;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  pid = atoi (args);
-
-  if (pid == getpid ())		/* Trying to masturbate? */
-    error (_("I refuse to debug myself!"));
+  pid = parse_pid (args);
 
   if (from_tty)
     {
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -426,6 +426,8 @@ int compare_positive_ints (const void *ap, const void *bp);
 
 extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
 
+extern unsigned long parse_pid (char *args);
+
 /* From demangle.c */
 
 extern void set_demangling_style (char *);
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2139,13 +2139,7 @@ gnu_attach (struct target_ops *ops, char *args, int from_tty)
   struct inf *inf = cur_inf ();
   struct inferior *inferior;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  pid = atoi (args);
-
-  if (pid == getpid ())		/* Trying to masturbate? */
-    error (_("I refuse to debug myself!"));
+  pid = parse_pid (args);
 
   if (from_tty)
     {
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -187,20 +187,9 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty)
 {
   char *exec_file;
   pid_t pid;
-  char *dummy;
   struct inferior *inf;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  dummy = args;
-  pid = strtol (args, &dummy, 0);
-  /* Some targets don't set errno on errors, grrr!  */
-  if (pid == 0 && args == dummy)
-    error (_("Illegal process-id: %s."), args);
-
-  if (pid == getpid ())		/* Trying to masturbate?  */
-    error (_("I refuse to debug myself!"));
+  pid = parse_pid (args);
 
   if (from_tty)
     {
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -691,20 +691,10 @@ inf_ttrace_attach (struct target_ops *ops, char *args, int from_tty)
 {
   char *exec_file;
   pid_t pid;
-  char *dummy;
   ttevent_t tte;
   struct inferior *inf;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  dummy = args;
-  pid = strtol (args, &dummy, 0);
-  if (pid == 0 && args == dummy)
-    error (_("Illegal process-id: %s."), args);
-
-  if (pid == getpid ())		/* Trying to masturbate?  */
-    error (_("I refuse to debug myself!"));
+  pid = parse_pid (args);
 
   if (from_tty)
     {
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -619,13 +619,7 @@ procfs_attach (struct target_ops *ops, char *args, int from_tty)
   int pid;
   struct inferior *inf;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  pid = atoi (args);
-
-  if (pid == getpid ())
-    error (_("Attaching GDB to itself is not a good idea..."));
+  pid = parse_pid (args);
 
   if (from_tty)
     {
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -3606,12 +3606,7 @@ procfs_attach (struct target_ops *ops, char *args, int from_tty)
   char *exec_file;
   int   pid;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  pid = atoi (args);
-  if (pid == getpid ())
-    error (_("Attaching GDB to itself is not a good idea..."));
+  pid = parse_pid (args);
 
   if (from_tty)
     {
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3857,17 +3857,9 @@ extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty)
 {
   struct remote_state *rs = get_remote_state ();
   int pid;
-  char *dummy;
   char *wait_status = NULL;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
-
-  dummy = args;
-  pid = strtol (args, &dummy, 0);
-  /* Some targets don't set errno on errors, grrr!  */
-  if (pid == 0 && args == dummy)
-    error (_("Illegal process-id: %s."), args);
+  pid = parse_pid (args);
 
   if (remote_protocol_packets[PACKET_vAttach].support == PACKET_DISABLE)
     error (_("This target does not support attaching to a process"));
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3648,6 +3648,32 @@ gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
   return ret;
 }
 
+/* Return ARGS parsed as numeric pid.  Call error if ARGS is not a valid number
+   or if ARGS is GDB's GETPID.  */
+
+unsigned long
+parse_pid (char *args)
+{
+  unsigned long pid;
+  char *dummy;
+
+  if (!args)
+    error_no_arg (_("process-id"));
+
+  dummy = args;
+  pid = strtoul (args, &dummy, 0);
+  /* Some targets don't set errno on errors, grrr!  */
+  if ((pid == 0 && dummy == args) || dummy != &args[strlen (args)])
+    error (_("Illegal process-id: %s."), args);
+
+#ifdef HAVE_GETPID
+  if (pid == getpid ())		/* Trying to masturbate?  */
+    error (_("I refuse to debug myself!"));
+#endif
+
+  return pid;
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_utils;
 
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1691,8 +1691,7 @@ windows_attach (struct target_ops *ops, char *args, int from_tty)
   BOOL ok;
   DWORD pid;
 
-  if (!args)
-    error_no_arg (_("process-id to attach"));
+  pid = parse_pid (args);
 
   if (set_process_privilege (SE_DEBUG_NAME, TRUE) < 0)
     {
@@ -1700,8 +1699,6 @@ windows_attach (struct target_ops *ops, char *args, int from_tty)
       printf_unfiltered ("This can cause attach to fail on Windows NT/2K/XP\n");
     }
 
-  pid = strtoul (args, 0, 0);		/* Windows pid */
-
   windows_init_thread_list ();
   ok = DebugActiveProcess (pid);
   saw_create = 0;
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -42,7 +42,7 @@ gdb_test "append binary value"  "Missing filename\."
 setup_xfail "mips-idt-*"
 send_gdb "attach\n"
 gdb_expect {
-    -re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
+    -re "Argument required .(process-id|program to attach).*$gdb_prompt $"\
 			{ pass "attach" }
     -re "You can't do that when your target is `None'.*$gdb_prompt $"\
 			{ pass "attach" }