[gdbserver] Use enum target_signal in _send_signal.

[gdbserver] Use enum target_signal in _send_signal.

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

Hi all,

This patch converts the send_signal function in target_ops to use enum 
target_signal
instead of the real target number.  I originally needed this for the 
WinCE gdbserver port.
Windows CE doesn't have signals support, and doesn't define SIGINT in 
any header.
Because of that, remote-utils.c wouldn't compile.  I still want to have 
support of stopping
a debuggee - this patch paves the way to that, with minimal interface 
changes.  I will handle
TARGET_SIGNAL_INT in the send_signal handler on the target side.

Ok?

Cheers,
Pedro Alves

---
gdb/gdbserver/ChangeLog:

2007-02-24  Pedro Alves  <pedro_alves@portugalmail.pt>

    * target.h (target_ops): Change parameter of send_signal from int to
    enum target_signal.
    * linux-low.c (linux_send_signal): Change parameter from int to
    enum target_signal.
    * spu-low.c (spu_send_signal): Likewise.
    * remote-utils.c (putpkt_binary): Pass TARGET_SIGNAL_INT instead
    of SIGINT.
    (input_interrupt): Likewise.


[-- Attachment #2: target_signal.diff --]
[-- Type: text/plain, Size: 2634 bytes --]

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2007-02-24 23:04:16.000000000 +0000
+++ src/gdb/gdbserver/linux-low.c	2007-02-24 23:05:54.000000000 +0000
@@ -1514,9 +1514,13 @@
 }
 
 static void
-linux_send_signal (int signum)
+linux_send_signal (enum target_signal sig)
 {
   extern unsigned long signal_pid;
+  int signum = target_signal_to_host (sig);
+
+  if (signum == 0)
+    return;
 
   if (cont_thread != 0 && cont_thread != -1)
     {
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2007-02-24 23:04:16.000000000 +0000
+++ src/gdb/gdbserver/spu-low.c	2007-02-24 23:05:54.000000000 +0000
@@ -551,8 +551,13 @@
 
 /* Send signal to inferior.  */
 static void
-spu_send_signal (int signo)
+spu_send_signal (enum target_signal sig)
 {
+  int signo = target_signal_to_host (sig);
+
+  if (signo == 0)
+    return;
+
   syscall (SYS_tkill, current_tid, signo);
 }
 
Index: src/gdb/gdbserver/target.h
===================================================================
--- src.orig/gdb/gdbserver/target.h	2007-02-24 23:04:16.000000000 +0000
+++ src/gdb/gdbserver/target.h	2007-02-24 23:05:54.000000000 +0000
@@ -128,7 +128,7 @@
   void (*look_up_symbols) (void);
 
   /* Send a signal to the inferior process, however is appropriate.  */
-  void (*send_signal) (int);
+  void (*send_signal) (enum target_signal);
 
   /* Read auxiliary vector data from the inferior process.
 
Index: src/gdb/gdbserver/remote-utils.c
===================================================================
--- src.orig/gdb/gdbserver/remote-utils.c	2007-02-24 23:04:20.000000000 +0000
+++ src/gdb/gdbserver/remote-utils.c	2007-02-24 23:05:54.000000000 +0000
@@ -549,7 +549,7 @@
 
       /* Check for an input interrupt while we're here.  */
       if (buf3[0] == '\003')
-	(*the_target->send_signal) (SIGINT);
+	(*the_target->send_signal) (TARGET_SIGNAL_INT);
     }
   while (buf3[0] != '+');
 
@@ -572,7 +572,7 @@
 /* Come here when we get an input interrupt from the remote side.  This
    interrupt should only be active while we are waiting for the child to do
    something.  About the only thing that should come through is a ^C, which
-   will cause us to send a SIGINT to the child.  */
+   will cause us to send a TARGET_SIGNAL_INT to the child.  */
 
 static void
 input_interrupt (int unused)
@@ -599,7 +599,7 @@
 	  return;
 	}
       
-      (*the_target->send_signal) (SIGINT);
+      (*the_target->send_signal) (TARGET_SIGNAL_INT);
     }
 }
 #endif

Re: [gdbserver] Use enum target_signal in _send_signal.

[Daniel Jacobowitz]

On Sat, Feb 24, 2007 at 11:23:56PM +0000, Pedro Alves wrote:
> Hi all,
> 
> This patch converts the send_signal function in target_ops to use enum 
> target_signal
> instead of the real target number.  I originally needed this for the 
> WinCE gdbserver port.
> Windows CE doesn't have signals support, and doesn't define SIGINT in 
> any header.
> Because of that, remote-utils.c wouldn't compile.  I still want to have 
> support of stopping
> a debuggee - this patch paves the way to that, with minimal interface 
> changes.  I will handle
> TARGET_SIGNAL_INT in the send_signal handler on the target side.
> 
> Ok?

Maybe this is the wrong abstraction entirely?  Because most signals
are passed to the continue / resume support, maybe the target method
here doesn't need a signal number at all.  We could replace it with
just send_interrupt.

What do you think?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [gdbserver] Use enum target_signal in _send_signal.

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

Daniel Jacobowitz escreveu:
> On Sat, Feb 24, 2007 at 11:23:56PM +0000, Pedro Alves wrote:
>> Hi all,
>>
>> This patch converts the send_signal function in target_ops to use enum 
>> target_signal
>> instead of the real target number.  I originally needed this for the 
>> WinCE gdbserver port.
>> Windows CE doesn't have signals support, and doesn't define SIGINT in 
>> any header.
>> Because of that, remote-utils.c wouldn't compile.  I still want to have 
>> support of stopping
>> a debuggee - this patch paves the way to that, with minimal interface 
>> changes.  I will handle
>> TARGET_SIGNAL_INT in the send_signal handler on the target side.
>>
>> Ok?
> 
> Maybe this is the wrong abstraction entirely?  Because most signals
> are passed to the continue / resume support, maybe the target method
> here doesn't need a signal number at all.  We could replace it with
> just send_interrupt.
> 
> What do you think?
> 


I agree.  I looked at shoehorning this into target_ops->(*resume)() with
TARGET_SIGNAL_INT, but that would be wrong, as resume expects an
already stopped process, and send_signal was used to do the actual
stopping.

How does this look?

Cheers,
Pedro Alves

---
gdb/gdbserver/ChangeLog:

    * target.h (target_ops): Rename send_signal to request_interrupt, and
    remove enum target_signal parameter.
    * linux-low.c (linux_request_interrupt): Rename from linux_send_signal,
    and always send SIGINT.
    * spu-low.c (spu_request_interrupt): Rename from spu_send_signal, and
    always send SIGINT.
    * remote-utils.c (putpkt_binary): Call request_interrupt, instead of
    send_signal.
    (input_interrupt): Likewise.


[-- Attachment #2: target_signal.diff --]
[-- Type: text/plain, Size: 3259 bytes --]

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2007-02-25 02:51:24.000000000 +0000
+++ src/gdb/gdbserver/linux-low.c	2007-02-25 02:56:42.000000000 +0000
@@ -1514,7 +1514,7 @@
 }
 
 static void
-linux_send_signal (int signum)
+linux_request_interrupt (void)
 {
   extern unsigned long signal_pid;
 
@@ -1523,10 +1523,10 @@
       struct process_info *process;
 
       process = get_thread_process (current_inferior);
-      kill_lwp (process->lwpid, signum);
+      kill_lwp (process->lwpid, SIGINT);
     }
   else
-    kill_lwp (signal_pid, signum);
+    kill_lwp (signal_pid, SIGINT);
 }
 
 /* Copy LEN bytes from inferior's auxiliary vector starting at OFFSET
@@ -1660,7 +1660,7 @@
   linux_read_memory,
   linux_write_memory,
   linux_look_up_symbols,
-  linux_send_signal,
+  linux_request_interrupt,
   linux_read_auxv,
   linux_insert_watchpoint,
   linux_remove_watchpoint,
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2007-02-25 02:51:24.000000000 +0000
+++ src/gdb/gdbserver/spu-low.c	2007-02-25 02:57:20.000000000 +0000
@@ -551,9 +551,9 @@
 
 /* Send signal to inferior.  */
 static void
-spu_send_signal (int signo)
+spu_request_interrupt (void)
 {
-  syscall (SYS_tkill, current_tid, signo);
+  syscall (SYS_tkill, current_tid, SIGINT);
 }
 
 static const char *
@@ -576,7 +576,7 @@
   spu_read_memory,
   spu_write_memory,
   spu_look_up_symbols,
-  spu_send_signal,
+  spu_request_interrupt,
   NULL,
   NULL,
   NULL,
Index: src/gdb/gdbserver/target.h
===================================================================
--- src.orig/gdb/gdbserver/target.h	2007-02-25 02:51:24.000000000 +0000
+++ src/gdb/gdbserver/target.h	2007-02-25 02:58:00.000000000 +0000
@@ -127,8 +127,10 @@
 
   void (*look_up_symbols) (void);
 
-  /* Send a signal to the inferior process, however is appropriate.  */
-  void (*send_signal) (int);
+  /* Send an interrupt request to the inferior process,
+     however is appropriate.  */
+
+  void (*request_interrupt) (void);
 
   /* Read auxiliary vector data from the inferior process.
 
Index: src/gdb/gdbserver/remote-utils.c
===================================================================
--- src.orig/gdb/gdbserver/remote-utils.c	2007-02-25 02:51:24.000000000 +0000
+++ src/gdb/gdbserver/remote-utils.c	2007-02-25 02:54:26.000000000 +0000
@@ -549,7 +549,7 @@
 
       /* Check for an input interrupt while we're here.  */
       if (buf3[0] == '\003')
-	(*the_target->send_signal) (SIGINT);
+	(*the_target->request_interrupt) ();
     }
   while (buf3[0] != '+');
 
@@ -572,7 +572,7 @@
 /* Come here when we get an input interrupt from the remote side.  This
    interrupt should only be active while we are waiting for the child to do
    something.  About the only thing that should come through is a ^C, which
-   will cause us to send a SIGINT to the child.  */
+   will cause us to request child interruption.  */
 
 static void
 input_interrupt (int unused)
@@ -599,7 +599,7 @@
 	  return;
 	}
       
-      (*the_target->send_signal) (SIGINT);
+      (*the_target->request_interrupt) ();
     }
 }
 #endif

Re: [gdbserver] Use enum target_signal in _send_signal.

[Daniel Jacobowitz]

On Sun, Feb 25, 2007 at 03:20:10AM +0000, Pedro Alves wrote:
> I agree.  I looked at shoehorning this into target_ops->(*resume)() with
> TARGET_SIGNAL_INT, but that would be wrong, as resume expects an
> already stopped process, and send_signal was used to do the actual
> stopping.
> 
> How does this look?

Great.  Thank you - this is OK to commit.

-- 
Daniel Jacobowitz
CodeSourcery