From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16883 invoked by alias); 12 Aug 2005 20:49:57 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 16862 invoked by uid 22791); 12 Aug 2005 20:49:54 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Fri, 12 Aug 2005 20:49:54 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id j7CKnqh9030187 for ; Fri, 12 Aug 2005 16:49:52 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id j7CKnqV16964 for ; Fri, 12 Aug 2005 16:49:52 -0400 Received: from localhost.localdomain (vpn50-47.rdu.redhat.com [172.16.50.47]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id j7CKnq97008252 for ; Fri, 12 Aug 2005 16:49:52 -0400 Received: from ironwood.lan (ironwood.lan [192.168.64.8]) by localhost.localdomain (8.12.11/8.12.10) with ESMTP id j7CKnk7S006627 for ; Fri, 12 Aug 2005 13:49:46 -0700 Date: Fri, 12 Aug 2005 20:52:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [RFC] Add support for Morpho ms1 processor Message-ID: <20050812134946.62ecd892@ironwood.lan> In-Reply-To: <200508121035.j7CAZs6D029649@elgar.sibelius.xs4all.nl> References: <20050603172038.449a41b9@ironwood.lan> <20050811165333.7d9d02f8@ironwood.lan> <200508121035.j7CAZs6D029649@elgar.sibelius.xs4all.nl> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SW-Source: 2005-08/txt/msg00141.txt.bz2 On Fri, 12 Aug 2005 12:35:54 +0200 (CEST) Mark Kettenis wrote: > > + if (regnum >= 0 && > > + regnum < sizeof (register_names) / sizeof (register_names[0])) > > + { > > + return register_names[regnum]; > > + } > > + else > > + internal_error (__FILE__, __LINE__, > > + _("ms1_register_name: illegal register number %d"), regnum); > > +} > > Please consider usin ARRAY_SIZE. I'd also use gdb_assert() here > instead of the excplicit internal_error(), but that's a bit of a > personal preference I assume. Please use invalid instead of illegal. I'm okay with gdb_assert(). That allows us to condense the above bit of code to just: gdb_assert (regnum >= 0 && regnum < ARRAY_SIZE (register_names)); return register_names[regnum]; The other benefit to using gdb_assert() is that there's one less string for the i18n translators to worry about. > > +static const unsigned char * > > +ms1_breakpoint_from_pc (CORE_ADDR *bp_addr, int *bp_size) > > +{ > > + static char breakpoint[] = { 0x68, 0, 0, 0 }; > > + > > + *bp_size = 4; > > + return breakpoint; > > +} > > Please use gdb_byte here too. Done. (I've changed the return type of ms1_breakpoint_from_pc() and the declaration of breakpoint[] in my sources.) > Otherwise this looks great! Thank you for reviewing this patch. Kevin