[RFA] change gdbserver's pids to int

[RFA] change gdbserver's pids to int

[Doug Evans]

gdb uses an int for a pid (see ptid.pid in defs.h),
and, for example, gdbserver's target_ops.create_inferior returns an int.
For consistency I made pid an int elsewhere in gdbserver,
except for "id" in struct inferior_list - it's used for more than just pids
although I suspect int could be used here too, left for another day.

Plus this cleans things up by removing local decls of signal_pid.

Ok to check in?

2008-12-03  Doug Evans  <dje@google.com>

	* target.h (struct target_ops, attach): Change type of pid arg to int.
	* server.c (signal_pid): Change to int from unsigned long.
	(start_inferior): Update.
	* server.h (signal_pid): Declare.
	* linux-low.c (linux_attach_lwp): Change type of pid arg to int.
	(linux_attach): Ditto.
	(linux_join): Remove decl of signal_pid.
	(linux_request_interrupt): Ditto.
	* win32-low.c (win32_join): Ditto.
	(win32_attach): Change type of pid arg to int.
	* inferiors.c (add_pid_to_list): Ditto.
	(pull_pid_from_list): Ditto.
	* spu-low.c (spu_attach): Ditto.

Index: inferiors.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/inferiors.c,v
retrieving revision 1.15
diff -u -p -r1.15 inferiors.c
--- inferiors.c	3 May 2008 17:16:43 -0000	1.15
+++ inferiors.c	4 Dec 2008 01:14:11 -0000
@@ -310,7 +310,7 @@ clear_inferiors (void)
    PID listing.  */
 
 void
-add_pid_to_list (struct inferior_list *list, unsigned long pid)
+add_pid_to_list (struct inferior_list *list, int pid)
 {
   struct inferior_list_entry *new_entry;
 
@@ -320,7 +320,7 @@ add_pid_to_list (struct inferior_list *l
 }
 
 int
-pull_pid_from_list (struct inferior_list *list, unsigned long pid)
+pull_pid_from_list (struct inferior_list *list, int pid)
 {
   struct inferior_list_entry *new_entry;
 
Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.82
diff -u -p -r1.82 linux-low.c
--- linux-low.c	2 Dec 2008 07:57:37 -0000	1.82
+++ linux-low.c	4 Dec 2008 01:14:12 -0000
@@ -307,7 +307,7 @@ linux_create_inferior (char *program, ch
 /* Attach to an inferior process.  */
 
 void
-linux_attach_lwp (unsigned long pid)
+linux_attach_lwp (int pid)
 {
   struct process_info *new_process;
 
@@ -316,14 +316,14 @@ linux_attach_lwp (unsigned long pid)
       if (all_threads.head != NULL)
 	{
 	  /* If we fail to attach to an LWP, just warn.  */
-	  fprintf (stderr, "Cannot attach to process %ld: %s (%d)\n", pid,
+	  fprintf (stderr, "Cannot attach to process %d: %s (%d)\n", pid,
 		   strerror (errno), errno);
 	  fflush (stderr);
 	  return;
 	}
       else
 	/* If we fail to attach to a process, report an error.  */
-	error ("Cannot attach to process %ld: %s (%d)\n", pid,
+	error ("Cannot attach to process %d: %s (%d)\n", pid,
 	       strerror (errno), errno);
     }
 
@@ -348,7 +348,7 @@ linux_attach_lwp (unsigned long pid)
 }
 
 int
-linux_attach (unsigned long pid)
+linux_attach (int pid)
 {
   struct process_info *process;
 
@@ -461,7 +461,6 @@ linux_detach (void)
 static void
 linux_join (void)
 {
-  extern unsigned long signal_pid;
   int status, ret;
 
   do {
@@ -1928,8 +1927,6 @@ linux_look_up_symbols (void)
 static void
 linux_request_interrupt (void)
 {
-  extern unsigned long signal_pid;
-
   if (cont_thread != 0 && cont_thread != -1)
     {
       struct process_info *process;
Index: linux-low.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.h,v
retrieving revision 1.23
diff -u -p -r1.23 linux-low.h
--- linux-low.h	28 Feb 2008 05:54:09 -0000	1.23
+++ linux-low.h	4 Dec 2008 01:14:12 -0000
@@ -140,7 +140,7 @@ struct process_info
 
 extern struct inferior_list all_processes;
 
-void linux_attach_lwp (unsigned long pid);
+void linux_attach_lwp (int pid);
 
 int thread_db_init (int use_events);
 int thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset,
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.81
diff -u -p -r1.81 server.c
--- server.c	2 Dec 2008 07:57:37 -0000	1.81
+++ server.c	4 Dec 2008 01:14:12 -0000
@@ -60,8 +60,7 @@ const char *gdbserver_xmltarget;
    send signals to the process when GDB sends us an asynchronous interrupt
    (user hitting Control-C in the client), and to wait for the child to exit
    when no longer debugging it.  */
-
-unsigned long signal_pid;
+int signal_pid;
 
 #ifdef SIGTTOU
 /* A file descriptor for the controlling terminal.  */
@@ -125,7 +124,7 @@ start_inferior (char **argv, char *statu
 
   /* FIXME: we don't actually know at this point that the create
      actually succeeded.  We won't know that until we wait.  */
-  fprintf (stderr, "Process %s created; pid = %ld\n", argv[0],
+  fprintf (stderr, "Process %s created; pid = %d\n", argv[0],
 	   signal_pid);
   fflush (stderr);
 
Index: server.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
retrieving revision 1.47
diff -u -p -r1.47 server.h
--- server.h	2 Dec 2008 07:57:37 -0000	1.47
+++ server.h	4 Dec 2008 01:14:12 -0000
@@ -141,8 +141,8 @@ void *inferior_target_data (struct threa
 void set_inferior_target_data (struct thread_info *, void *);
 void *inferior_regcache_data (struct thread_info *);
 void set_inferior_regcache_data (struct thread_info *, void *);
-void add_pid_to_list (struct inferior_list *list, unsigned long pid);
-int pull_pid_from_list (struct inferior_list *list, unsigned long pid);
+void add_pid_to_list (struct inferior_list *list, int pid);
+int pull_pid_from_list (struct inferior_list *list, int pid);
 
 void loaded_dll (const char *name, CORE_ADDR base_addr);
 void unloaded_dll (const char *name, CORE_ADDR base_addr);
@@ -157,9 +157,8 @@ extern unsigned long old_thread_from_wai
 extern int server_waiting;
 extern int debug_threads;
 extern int pass_signals[];
-
 extern jmp_buf toplevel;
-
+extern int signal_pid;
 extern int disable_packet_vCont;
 extern int disable_packet_Tthread;
 extern int disable_packet_qC;
Index: spu-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/spu-low.c,v
retrieving revision 1.17
diff -u -p -r1.17 spu-low.c
--- spu-low.c	28 Feb 2008 05:54:09 -0000	1.17
+++ spu-low.c	4 Dec 2008 01:14:12 -0000
@@ -295,7 +295,7 @@ spu_create_inferior (char *program, char
 
 /* Attach to an inferior process.  */
 int
-spu_attach (unsigned long  pid)
+spu_attach (int pid)
 {
   if (ptrace (PTRACE_ATTACH, pid, 0, 0) != 0)
     {
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
retrieving revision 1.29
diff -u -p -r1.29 target.h
--- target.h	2 Dec 2008 07:57:37 -0000	1.29
+++ target.h	4 Dec 2008 01:14:12 -0000
@@ -62,7 +62,7 @@ struct target_ops
      Returns -1 if attaching is unsupported, 0 on success, and calls
      error() otherwise.  */
 
-  int (*attach) (unsigned long pid);
+  int (*attach) (int pid);
 
   /* Kill all inferiors.  */
 
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.27
diff -u -p -r1.27 win32-low.c
--- win32-low.c	28 Feb 2008 05:54:09 -0000	1.27
+++ win32-low.c	4 Dec 2008 01:14:12 -0000
@@ -588,7 +588,7 @@ win32_create_inferior (char *program, ch
    PID is the process ID to attach to, specified by the user
    or a higher layer.  */
 static int
-win32_attach (unsigned long pid)
+win32_attach (int pid)
 {
   HANDLE h;
   winapi_DebugSetProcessKillOnExit DebugSetProcessKillOnExit = NULL;
@@ -743,8 +743,6 @@ win32_detach (void)
 static void
 win32_join (void)
 {
-  extern unsigned long signal_pid;
-
   HANDLE h = OpenProcess (PROCESS_ALL_ACCESS, FALSE, signal_pid);
   if (h != NULL)
     {

Re: [RFA] change gdbserver's pids to int

[Daniel Jacobowitz]

On Wed, Dec 03, 2008 at 05:24:16PM -0800, Doug Evans wrote:
> gdb uses an int for a pid (see ptid.pid in defs.h),
> and, for example, gdbserver's target_ops.create_inferior returns an int.
> For consistency I made pid an int elsewhere in gdbserver,
> except for "id" in struct inferior_list - it's used for more than just pids
> although I suspect int could be used here too, left for another day.
> 
> Plus this cleans things up by removing local decls of signal_pid.
> 
> Ok to check in?

Are you sure that every place you touched gets a system PID, not a
thread ID?  They used to be ints, but were changed to unsigned long
because NPTL's TIDs do not fit in an int.

Also, do Windows PIDs fit in an int?  Win32 pids must, but I expect
we'll get a Win64 port at some point.

I'd like to know the advantage before moving all the deck chairs round
again.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] change gdbserver's pids to int

[Doug Evans]

On Thu, Dec 4, 2008 at 5:15 AM, Daniel Jacobowitz <drow@false.org> wrote:
> On Wed, Dec 03, 2008 at 05:24:16PM -0800, Doug Evans wrote:
>> gdb uses an int for a pid (see ptid.pid in defs.h),
>> and, for example, gdbserver's target_ops.create_inferior returns an int.
>> For consistency I made pid an int elsewhere in gdbserver,
>> except for "id" in struct inferior_list - it's used for more than just pids
>> although I suspect int could be used here too, left for another day.
>>
>> Plus this cleans things up by removing local decls of signal_pid.
>>
>> Ok to check in?
>
> Are you sure that every place you touched gets a system PID, not a
> thread ID?  They used to be ints, but were changed to unsigned long
> because NPTL's TIDs do not fit in an int.

Ya, I specifically stuck to pids.

> Also, do Windows PIDs fit in an int?  Win32 pids must, but I expect
> we'll get a Win64 port at some point.

Good question.  I was in part going for consistency with what gdb uses.
Maybe it would be useful to have gdb_pid_t, gdb_tid_t types that both
gdb and gdbserver use.

> I'd like to know the advantage before moving all the deck chairs round
> again.

Consistency with gdb is good.  Plus it's odd to pass an unsigned long
to linux_attach_lwp and then it pass that as the pid argument to
ptrace when linux_create_inferior is using an int for pid.  And it's
odd that signal_pid is an unsigned long which is then passed to
waitpid.  Compiling gdbserver with -Wconversion may have a lot of
noise (enough to warrant not making it the default anyway), but I
think we can still make an effort to be clean.

Re: [RFA] change gdbserver's pids to int

[Daniel Jacobowitz]

On Thu, Dec 04, 2008 at 03:02:05PM -0800, Doug Evans wrote:
> > Are you sure that every place you touched gets a system PID, not a
> > thread ID?  They used to be ints, but were changed to unsigned long
> > because NPTL's TIDs do not fit in an int.
> 
> Ya, I specifically stuck to pids.

A lot of these places used to be thread IDs.  At some point we
stopped reporting those to GDB, to avoid the same sort of problem.

> > Also, do Windows PIDs fit in an int?  Win32 pids must, but I expect
> > we'll get a Win64 port at some point.
> 
> Good question.  I was in part going for consistency with what gdb uses.
> Maybe it would be useful to have gdb_pid_t, gdb_tid_t types that both
> gdb and gdbserver use.

I don't personally think consistency between gdb and gdbserver is all
that big of a goal.  There's lots of similar code between gdb and
gdbserver, but even if we fixed up common interfaces I think we'd have
to add a lot of gunk to gdbserver to be able to entirely share
anything; we already have some such gunk for signals.c.

Switching them to some typedef to int sounds OK to me.  I admit I
don't see the point, but I wouldn't reject the patch.  Dumping them
all back to int just makes them less opaque than they are now though.

-- 
Daniel Jacobowitz
CodeSourcery