From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30124 invoked by alias); 6 Jun 2012 22:52:40 -0000 Received: (qmail 30105 invoked by uid 22791); 6 Jun 2012 22:52:38 -0000 X-SWARE-Spam-Status: No, hits=-4.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 06 Jun 2012 22:52:23 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1ScP5R-00027t-5U from Maciej_Rozycki@mentor.com ; Wed, 06 Jun 2012 15:52:21 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Wed, 6 Jun 2012 15:52:20 -0700 Received: from [172.30.1.189] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Wed, 6 Jun 2012 23:52:18 +0100 Date: Wed, 06 Jun 2012 22:52:00 -0000 From: "Maciej W. Rozycki" To: Michael Eager CC: "gdb-patches@sourceware.org" , Pedro Alves Subject: Re: [PATCH] MIPS Linux signals In-Reply-To: <4FC947AE.2000008@eagerm.com> Message-ID: References: <4FC9244F.1080704@eagerm.com> <4FC947AE.2000008@eagerm.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" 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: 2012-06/txt/msg00207.txt.bz2 On Fri, 1 Jun 2012, Michael Eager wrote: > Some of the awkward code in translating REALTIME signals is due to > the fact that the GDB_SIGNAL_REALTIME block is not contiguous. OK, fair enough, it's always good to include any such explanations with patch submissions so as to make the review process easier for the reviewers. It makes sense to note the peculiarity around code concerned too. > I recast the section of code copied from signals.c in terms of > MIPS_SIGRTMIN & MIPS_SIGRTMAX, but it isn't clear to me that this > is an improvement, since it is now different from the similar code > in signals.c. This is target-specific code, its goal is not to be as close as to generic code as possible, but (as with any code, except where critical performance or any other requirements mandate otherwise) straightforward and matching the requirements of the specific target concerned. > Index: gdb/ChangeLog > =================================================================== > RCS file: /cvs/src/src/gdb/ChangeLog,v > retrieving revision 1.14309 > diff -u -p -r1.14309 ChangeLog > --- gdb/ChangeLog 1 Jun 2012 16:37:56 -0000 1.14309 > +++ gdb/ChangeLog 1 Jun 2012 22:43:51 -0000 > @@ -1,3 +1,8 @@ > +2012-06-01 Michael Eager > + > + * mips-linux-tdep.c (mips_gdb_signal_from_target): New > + * mips-linux-tdep.h (mips_signals): New > + > 2012-06-01 Siddhesh Poyarekar > > * target.c (target_read_memory): Make LEN argument as size_t. Please supply ChangeLog entries separately, they're not going to work when included as a diff and will require hand-editing anyway. > Index: gdb/mips-linux-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v > retrieving revision 1.94 > diff -u -p -r1.94 mips-linux-tdep.c > --- gdb/mips-linux-tdep.c 22 May 2012 17:12:07 -0000 1.94 > +++ gdb/mips-linux-tdep.c 1 Jun 2012 22:43:51 -0000 > @@ -1330,6 +1331,98 @@ mips_linux_get_syscall_number (struct gd [...] > + > + if (signo >= MIPS_SIGRTMIN && signo <= MIPS_SIGRTMAX) > + { > + /* This block of GDB_SIGNAL_REALTIME value is in order. */ > + if (MIPS_SIGRTMIN <= signo && signo <= (MIPS_SIGRTMIN + 32)) > + return ((enum gdb_signal) > + (signo - MIPS_SIGRTMIN + 1 + (int) GDB_SIGNAL_REALTIME_33)); > + else if (signo == MIPS_SIGRTMIN) > + return GDB_SIGNAL_REALTIME_32; > + else if ((MIPS_SIGRTMIN + 32) <= signo && signo <= MIPS_SIGRTMAX) > + return ((enum gdb_signal) > + (signo - (MIPS_SIGRTMIN + 32) + (int) GDB_SIGNAL_REALTIME_64)); > + else > + error ("GDB bug: unrecognized real-time signal"); > + } > + > + return GDB_SIGNAL_UNKNOWN; > +} > + > /* Initialize one of the GNU/Linux OS ABIs. */ > > static void I find this unreadable (and buggy too, which is likely a consequence), per observations above please rewrite this as below: if (signo >= MIPS_SIGRTMIN && signo <= MIPS_SIGRTMAX) { /* GDB_SIGNAL_REALTIME values are not contiguous, map parts of the MIPS block to the respective GDB_SIGNAL_REALTIME blocks. */ signo -= MIPS_SIGRTMIN; if (signo == 0) return GDB_SIGNAL_REALTIME_32; else if (signo < 32) return ((enum gdb_signal) (signo - 1 + (int) GDB_SIGNAL_REALTIME_33)); else return ((enum gdb_signal) (signo - 32 + (int) GDB_SIGNAL_REALTIME_64)); } return GDB_SIGNAL_UNKNOWN; } -- the casts are probably redundant, but let them stay. OK with this change, the rest is fine with me. Thanks for tackling this problem. Maciej