From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7651 invoked by alias); 6 Jun 2012 23:12:59 -0000 Received: (qmail 7643 invoked by uid 22791); 6 Jun 2012 23:12:57 -0000 X-SWARE-Spam-Status: No, hits=-2.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED X-Spam-Check-By: sourceware.org Received: from shell4.bayarea.net (HELO shell4.bayarea.net) (209.128.82.1) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 06 Jun 2012 23:12:44 +0000 Received: (qmail 5938 invoked from network); 6 Jun 2012 16:12:43 -0700 Received: from c-76-102-3-160.hsd1.ca.comcast.net (HELO redwood.eagercon.com) (76.102.3.160) by shell4.bayarea.net with SMTP; 6 Jun 2012 16:12:42 -0700 Message-ID: <4FCFE3EA.8080703@eagerm.com> Date: Wed, 06 Jun 2012 23:12:00 -0000 From: Michael Eager User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: "Maciej W. Rozycki" CC: "gdb-patches@sourceware.org" , Pedro Alves Subject: Re: [PATCH] MIPS Linux signals References: <4FC9244F.1080704@eagerm.com> <4FC947AE.2000008@eagerm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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: 2012-06/txt/msg00208.txt.bz2 On 06/06/2012 03:52 PM, Maciej W. Rozycki wrote: > 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. I forgot to edit the ChangeLog out of the diff. > >> 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; > } OK. > > -- the casts are probably redundant, but let them stay. > > OK with this change, the rest is fine with me. Thanks for tackling this > problem. Thanks. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077