This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] target_signal obvious fixups
On Mon, 06 Sep 2010 13:29:08 +0200, Pedro Alves wrote:
> On Saturday 04 September 2010 22:58:12, Jan Kratochvil wrote:
> Something like this would be better:
>
> enum target_signal sig = ((core_gdbarch != NULL)
> ? gdbarch_target_signal_from_host (core_gdbarch, siggy)
> : target_signal_from_host (siggy));
>
> printf_filtered (_("Program terminated with signal %d, %s.\n"), siggy,
> target_signal_to_string (sig));
OK, used it.
> > --- a/gdb/linux-nat.c
> > +++ b/gdb/linux-nat.c
> > - signo ? strsignal (signo) : "0");
> > + target_signal_to_name (signo));
>
> I think it'd be better to keep using strsignal, so we may easily see
> when the target_signal translates to something bizarre that we're about
> to send to ptrace/tkill/kill. (This is at the native level, so it goes
> along with the PTRACE_SINGLESTEP|CONT.) Agree? See my patch.
I would like to make a note target_signal_to_host used for native target would
do anyway
warning ("Signal %s does not exist on this system.\n",
target_signal_to_name (oursig));
But I understand even for a supported signal target_signal_to_name and
strsignal can have different output.
Therefore used your variant (from the other patch).
> > --- a/gdb/gdbserver/server.c
> > +++ b/gdb/gdbserver/server.c
> > @@ -1748,7 +1748,8 @@ handle_v_cont (char *own_buf)
> > {
> > - int sig;
> > + enum target_signal sig;
> > +
> > sig = strtol (p + 1, &q, 16);
>
> This is okay with me, but I'd like to point out that
> this makes one use of sig correct (a bit below this code,
> not quoted), while making the "sig = strtol" assignment
> assume you can convert a long to an enum / target_signal.
> IMO, this is a case where target_signal_from_number would
> make sense (I had one in my patch, not sure you had one
> in yours). Let me repeat this is okay with me as is...
I am sorry but this part wasn't appropriate for this kind of patch. SIG needs
to be both `int' and `enum target_signal' so it is a subject for the "real
patch" dependent variants of conversions.
Dropped this patch chunk.
Checked-in.
Thanks,
Jan
http://sourceware.org/ml/gdb-cvs/2010-09/msg00044.html
--- src/gdb/ChangeLog 2010/09/06 13:34:01 1.12154
+++ src/gdb/ChangeLog 2010/09/06 13:59:00 1.12155
@@ -1,3 +1,18 @@
+2010-09-06 Jan Kratochvil <jan.kratochvil@redhat.com>
+ Pedro Alves <pedro@codesourcery.com>
+
+ * corelow.c (core_open): Use target_signal_from_host if CORE_GDBARCH
+ is NULL.
+ * fork-child.c (startup_inferior) <resume_signal>: Use enum
+ target_signal type.
+ * linux-nat.c (linux_nat_resume): Use target_signal_to_host before
+ calling strsignal. Use enum target_signal type for saved_signo.
+ (linux_handle_extended_wait) <signo>: Use enum target_signal type.
+ (linux_nat_wait_1): Use enum target_signal type for signo. Use
+ target_signal_to_host before calling strsignal.
+ * remote-m32r-sdi.c (m32r_wait, m32r_detach): Replace 0 by
+ TARGET_SIGNAL_0.
+
2010-09-06 Pedro Alves <pedro@codesourcery.com>
Jan Kratochvil <jan.kratochvil@redhat.com>
--- src/gdb/gdbserver/ChangeLog 2010/09/06 10:43:58 1.424
+++ src/gdb/gdbserver/ChangeLog 2010/09/06 13:59:02 1.425
@@ -1,3 +1,7 @@
+2010-09-06 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * target.c (mywait) <TARGET_WAITKIND_EXITED>: Fix to use INTEGER.
+
2010-09-06 Pedro Alves <pedro@codesourcery.com>
* Makefile.in (install-only): Replace $IPA_DEPFILES with
--- src/gdb/corelow.c 2010/08/18 12:24:12 1.103
+++ src/gdb/corelow.c 2010/09/06 13:59:02 1.104
@@ -429,15 +429,18 @@
siggy = bfd_core_file_failing_signal (core_bfd);
if (siggy > 0)
- /* NOTE: target_signal_from_host() converts a target signal value
- into gdb's internal signal value. Unfortunately gdb's internal
- value is called ``target_signal'' and this function got the
- name ..._from_host(). */
- printf_filtered (_("Program terminated with signal %d, %s.\n"), siggy,
- target_signal_to_string (
- (core_gdbarch != NULL) ?
- gdbarch_target_signal_from_host (core_gdbarch, siggy)
- : siggy));
+ {
+ /* NOTE: target_signal_from_host() converts a target signal value
+ into gdb's internal signal value. Unfortunately gdb's internal
+ value is called ``target_signal'' and this function got the
+ name ..._from_host(). */
+ enum target_signal sig = (core_gdbarch != NULL
+ ? gdbarch_target_signal_from_host (core_gdbarch, siggy)
+ : target_signal_from_host (siggy));
+
+ printf_filtered (_("Program terminated with signal %d, %s.\n"), siggy,
+ target_signal_to_string (sig));
+ }
/* Fetch all registers from core file. */
target_fetch_registers (get_current_regcache (), -1);
--- src/gdb/fork-child.c 2010/05/14 19:27:05 1.56
+++ src/gdb/fork-child.c 2010/09/06 13:59:02 1.57
@@ -448,7 +448,7 @@
while (1)
{
- int resume_signal = TARGET_SIGNAL_0;
+ enum target_signal resume_signal = TARGET_SIGNAL_0;
ptid_t event_ptid;
struct target_waitstatus ws;
--- src/gdb/linux-nat.c 2010/09/02 01:19:32 1.183
+++ src/gdb/linux-nat.c 2010/09/06 13:59:02 1.184
@@ -1874,7 +1874,8 @@
"LLR: Preparing to %s %s, %s, inferior_ptid %s\n",
step ? "step" : "resume",
target_pid_to_str (ptid),
- signo ? strsignal (signo) : "0",
+ (signo != TARGET_SIGNAL_0
+ ? strsignal (target_signal_to_host (signo)) : "0"),
target_pid_to_str (inferior_ptid));
block_child_signals (&prev_mask);
@@ -1907,7 +1908,7 @@
if (lp->status && WIFSTOPPED (lp->status))
{
- int saved_signo;
+ enum target_signal saved_signo;
struct inferior *inf;
inf = find_inferior_pid (ptid_get_pid (lp->ptid));
@@ -1974,7 +1975,8 @@
"LLR: %s %s, %s (resume event thread)\n",
step ? "PTRACE_SINGLESTEP" : "PTRACE_CONT",
target_pid_to_str (ptid),
- signo ? strsignal (signo) : "0");
+ (signo != TARGET_SIGNAL_0
+ ? strsignal (target_signal_to_host (signo)) : "0"));
restore_child_signals_mask (&prev_mask);
if (target_can_async_p ())
@@ -2266,7 +2268,7 @@
catchpoints. */
if (!stopping)
{
- int signo;
+ enum target_signal signo;
new_lp->stopped = 0;
new_lp->resumed = 1;
@@ -3567,7 +3569,7 @@
if (WIFSTOPPED (status))
{
- int signo = target_signal_from_host (WSTOPSIG (status));
+ enum target_signal signo = target_signal_from_host (WSTOPSIG (status));
struct inferior *inf;
inf = find_inferior_pid (ptid_get_pid (lp->ptid));
@@ -3597,7 +3599,9 @@
lp->step ?
"PTRACE_SINGLESTEP" : "PTRACE_CONT",
target_pid_to_str (lp->ptid),
- signo ? strsignal (signo) : "0");
+ (signo != TARGET_SIGNAL_0
+ ? strsignal (target_signal_to_host (signo))
+ : "0"));
lp->stopped = 0;
goto retry;
}
--- src/gdb/remote-m32r-sdi.c 2010/07/07 16:15:16 1.51
+++ src/gdb/remote-m32r-sdi.c 2010/09/06 13:59:02 1.52
@@ -715,7 +715,7 @@
fprintf_unfiltered (gdb_stdlog, "m32r_wait()\n");
status->kind = TARGET_WAITKIND_EXITED;
- status->value.sig = 0;
+ status->value.sig = TARGET_SIGNAL_0;
interrupted = 0;
prev_sigint = signal (SIGINT, gdb_cntrl_c);
@@ -886,7 +886,7 @@
if (remote_debug)
fprintf_unfiltered (gdb_stdlog, "m32r_detach(%d)\n", from_tty);
- m32r_resume (ops, inferior_ptid, 0, 0);
+ m32r_resume (ops, inferior_ptid, 0, TARGET_SIGNAL_0);
/* calls m32r_close to do the real work */
pop_target ();
--- src/gdb/gdbserver/target.c 2010/09/01 01:53:43 1.19
+++ src/gdb/gdbserver/target.c 2010/09/06 13:59:03 1.20
@@ -98,7 +98,7 @@
if (ourstatus->kind == TARGET_WAITKIND_EXITED)
fprintf (stderr,
- "\nChild exited with status %d\n", ourstatus->value.sig);
+ "\nChild exited with status %d\n", ourstatus->value.integer);
else if (ourstatus->kind == TARGET_WAITKIND_SIGNALLED)
fprintf (stderr, "\nChild terminated with signal = 0x%x (%s)\n",
target_signal_to_host (ourstatus->value.sig),