From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3227 invoked by alias); 14 Jun 2013 17:58:45 -0000 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 Received: (qmail 3214 invoked by uid 89); 14 Jun 2013 17:58:44 -0000 X-Spam-SWARE-Status: No, score=-6.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 14 Jun 2013 17:58:44 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5EHwdFf000580 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 14 Jun 2013 13:58:40 -0400 Received: from psique (ovpn-113-133.phx2.redhat.com [10.3.113.133]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r5EHwaCo029033 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 14 Jun 2013 13:58:38 -0400 From: Sergio Durigan Junior To: "Pierre Muller" Cc: "'GDB Patches'" Subject: Re: [RFC/PATCH] Add new internal variable $_signo References: <002801ce68dd$845cd2e0$8d1678a0$@muller@ics-cnrs.unistra.fr> X-URL: http://www.redhat.com Date: Fri, 14 Jun 2013 17:59:00 -0000 In-Reply-To: <002801ce68dd$845cd2e0$8d1678a0$@muller@ics-cnrs.unistra.fr> (Pierre Muller's message of "Fri, 14 Jun 2013 10:59:44 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-06/txt/msg00339.txt.bz2 Hi Pierre, Thanks for the review. On Friday, June 14 2013, Pierre Muller wrote: > Is it that I didn't understand the patch correctly or > do you use the GDB signal number in infrun.c > while you use the native signal integer value in the > corelow.c case? Yes, you are right. > Aren't those two values sometimes different? They probably are in some cases. > Wouldn't it be more consistent to only use the GDB internal number? Hm, now that you raised the question, I am wondering. I believe it is more consistent to use the GDB internal number when we are printing something, yeah. However, in the $_signo case, we are actually displaying the number itself, so your comment applies to my patch, but backwards: I should actually be converting the GDB internal number to the actual signal number on infrun.c. > In fact, this "inconsistency" is not specific to your patch, > the siggy from corelow.c is printed out, while other signals are always > first converted to GDB enum values before being printed (and apparently not > in > integer form but using the gdb_signal_to_name function. > > Shouldn't we use gdb_signal_to_name (sig) in core_open > and set $_signo also to sig? I don't think $_signo should be set to "sig", it should remain "siggy". What should happen (IIUC everything) is that the infrun.c uses should be converted to the actual signal number (by using gdb_signal_to_host). > Proposed patch (untested...) > Should I submit it independently or > is there a specific reason to print the numeric value of the signal > for core dumps while we seem to use signal names elsewhere? I always think that the numeric value of a signal is a good thing to be printed. > 2013-06-14 Pierre Muller > > * corelow.c (core_open): Use GDB signal name instead of raw > signal value. > > Index: corelow.c > =================================================================== > RCS file: /cvs/src/src/gdb/corelow.c,v > retrieving revision 1.132 > diff -u -p -r1.132 corelow.c > --- corelow.c 15 May 2013 12:26:14 -0000 1.132 > +++ corelow.c 14 Jun 2013 08:56:08 -0000 > @@ -445,7 +445,7 @@ core_open (char *filename, int from_tty) > : gdb_signal_from_host (siggy)); > > printf_filtered (_("Program terminated with signal %s, %s.\n"), Could you print the number here? BTW, the number is "siggy" :-). You could say: printf_filtered (_("Program terminated with signal %s (%d), %s.\n"), BTW, this patch is wrong for me, the previous printf_filtered line was "%d, %s". > - siggy, gdb_signal_to_string (sig)); > + gdb_signal_to_name (sig), gdb_signal_to_string > (sig)); > } > > /* Fetch all registers from core file. */ I will send a new patch addressing the issues I mentioned above shortly. Thanks, -- Sergio