From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5313 invoked by alias); 6 Sep 2010 14:09:57 -0000 Received: (qmail 5295 invoked by uid 22791); 6 Sep 2010 14:09:54 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 06 Sep 2010 14:09:09 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o86E90PR016252 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 6 Sep 2010 10:09:00 -0400 Received: from host1.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o86E8vP6002006 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 6 Sep 2010 10:08:59 -0400 Received: from host1.dyn.jankratochvil.net (localhost [127.0.0.1]) by host1.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o86E8vfA022654; Mon, 6 Sep 2010 16:08:57 +0200 Received: (from jkratoch@localhost) by host1.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o86E8ubU022632; Mon, 6 Sep 2010 16:08:56 +0200 Date: Mon, 06 Sep 2010 14:54:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [patch] target_signal obvious fixups Message-ID: <20100906140855.GA3989@host1.dyn.jankratochvil.net> References: <20100904215812.GA9740@host1.dyn.jankratochvil.net> <201009061229.09019.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201009061229.09019.pedro@codesourcery.com> User-Agent: Mutt/1.5.20 (2009-12-10) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-09/txt/msg00157.txt.bz2 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 + Pedro Alves + + * corelow.c (core_open): Use target_signal_from_host if CORE_GDBARCH + is NULL. + * fork-child.c (startup_inferior) : 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) : 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 Jan Kratochvil --- 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 + + * target.c (mywait) : Fix to use INTEGER. + 2010-09-06 Pedro Alves * 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),