[patch] enum target_signal vs. int host_signal

[patch] enum target_signal vs. int host_signal

[Jan Kratochvil]

Hi,

enum target_signal and int host_signal are compatible without a cast in C.

Besides the change below I find some incorrect usage of gdb/gdbserver/target.h
struct thread_resume -> sig.  But one should decide first which kind of signal
it should be.

No regressions on x86_64-fedora13-linux-gnu for */*core*.exp.

I will post also sanity checking patch for this kind of error.


Thanks,
Jan


gdb/
2010-07-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-nat.c (linux_nat_do_thread_registers): Convert STOP_SIGNAL to
	the host signal first.

--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4190,7 +4190,8 @@ linux_nat_do_thread_registers (bfd *obfd, ptid_t ptid,
 	if (strcmp (sect_list->sect_name, ".reg") == 0)
 	  note_data = (char *) elfcore_write_prstatus
 				(obfd, note_data, note_size,
-				 lwp, stop_signal, gdb_regset);
+				 lwp, target_signal_to_host (stop_signal),
+				 gdb_regset);
 	else
 	  note_data = (char *) elfcore_write_register_note
 				(obfd, note_data, note_size,
@@ -4217,11 +4218,9 @@ linux_nat_do_thread_registers (bfd *obfd, ptid_t ptid,
       else
 	fill_gregset (regcache, &gregs, -1);
 
-      note_data = (char *) elfcore_write_prstatus (obfd,
-						   note_data,
-						   note_size,
-						   lwp,
-						   stop_signal, &gregs);
+      note_data = (char *) elfcore_write_prstatus
+	(obfd, note_data, note_size, lwp, target_signal_to_host (stop_signal),
+	 &gregs);
 
       if (core_regset_p
           && (regset = gdbarch_regset_from_core_section (gdbarch, ".reg2",

Re: [patch] enum target_signal vs. int host_signal

[Pedro Alves]

On Saturday 24 July 2010 12:42:08, Jan Kratochvil wrote:

> Besides the change below I find some incorrect usage of gdb/gdbserver/target.h
> struct thread_resume -> sig.  But one should decide first which kind of signal
> it should be.

Currently, host signal.  I too see one place in server.c that got that
mixed up.  And win32-low.c uses TARGET_SIGNAL_0 when it shouldn't.
I guess it would be a bit cleaner if thread_resume->sig was a
gdb signal instead, so that core gdbserver only handles gdb signals
(as is, there's a bit of a mix, since struct target_waitstatus holds
a gdb signal), though that's a larger change than just fixing the current
misuses.

> gdb/
> 2010-07-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* linux-nat.c (linux_nat_do_thread_registers): Convert STOP_SIGNAL to
> 	the host signal first.

Okay.

-- 
Pedro Alves

[patch] gdbserver: enum target_signal vs. int host_signal [Re: [patch] enum target_signal vs. int host_signal]

[Jan Kratochvil]

On Sun, 25 Jul 2010 00:44:39 +0200, Pedro Alves wrote:
> Currently, host signal.

Therefore OK to check-in?


> And win32-low.c uses TARGET_SIGNAL_0 when it shouldn't.

I haven't fixed this common kind of bug in this pre-patchset as
TARGET_SIGNAL_0 == 0 so there is no functionality bug.


> > gdb/
> > 2010-07-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* linux-nat.c (linux_nat_do_thread_registers): Convert STOP_SIGNAL to
> > 	the host signal first.
> 
> Okay.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-07/msg00144.html


Thanks,
Jan


gdb/gdbserver/
2010-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* server.c (handle_target_event): Use target_signal_to_host for
	resume_info.sig initialization.
	* target.h (struct thread_resume) <sig>: New comment.

--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3147,7 +3147,7 @@ handle_target_event (int err, gdb_client_data client_data)
 
 	      resume_info.thread = last_ptid;
 	      resume_info.kind = resume_continue;
-	      resume_info.sig = last_status.value.sig;
+	      resume_info.sig = target_signal_to_host (last_status.value.sig);
 	      (*the_target->resume) (&resume_info, 1);
 	    }
 	  else if (debug_threads)
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -53,7 +53,8 @@ struct thread_resume
   /* If non-zero, send this signal when we resume, or to stop the
      thread.  If stopping a thread, and this is 0, the target should
      stop the thread however it best decides to (e.g., SIGSTOP on
-     linux; SuspendThread on win32).  */
+     linux; SuspendThread on win32).  This is a host signal value (not
+     enum target_signal).  */
   int sig;
 };
 

Re: [patch] gdbserver: enum target_signal vs. int host_signal [Re: [patch] enum target_signal vs. int host_signal]

[Pedro Alves]

On Sunday 25 July 2010 10:33:34, Jan Kratochvil wrote:
> On Sun, 25 Jul 2010 00:44:39 +0200, Pedro Alves wrote:
> > Currently, host signal.
> 
> Therefore OK to check-in?

Yes, thanks.

-- 
Pedro Alves

Re: [patch] gdbserver: enum target_signal vs. int host_signal [Re: [patch] enum target_signal vs. int host_signal]

[Jan Kratochvil]

On Sun, 25 Jul 2010 12:05:02 +0200, Pedro Alves wrote:
> On Sunday 25 July 2010 10:33:34, Jan Kratochvil wrote:
> > On Sun, 25 Jul 2010 00:44:39 +0200, Pedro Alves wrote:
> > > Currently, host signal.
> > 
> > Therefore OK to check-in?
> 
> Yes, thanks.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-07/msg00145.html


Thanks,
Jan