From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14908 invoked by alias); 22 Nov 2012 17:50:28 -0000 Received: (qmail 14895 invoked by uid 22791); 22 Nov 2012 17:50:26 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 22 Nov 2012 17:50:20 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 20ACF2E1FD; Thu, 22 Nov 2012 12:50:20 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id CuCWq-o1mwFD; Thu, 22 Nov 2012 12:50:20 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id D3B512E1E6; Thu, 22 Nov 2012 12:50:19 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id A3CD9C2353; Thu, 22 Nov 2012 09:50:10 -0800 (PST) Date: Thu, 22 Nov 2012 17:50:00 -0000 From: Joel Brobecker To: Kaushik Phatak Cc: Yao Qi , "gdb-patches@sourceware.org" Subject: Re: [RFA 3/5] New port: CR16: gdb port Message-ID: <20121122175010.GG9964@adacore.com> References: <507279C7.8080401@codesourcery.com> <20121022224107.GB3713@adacore.com> <20121023135502.GA3555@adacore.com> <20121115174313.GC3790@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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-11/txt/msg00600.txt.bz2 > > + allWords = > > Can you please void using the mixed-cap style for variables? > This variable along with currInsn suffer from the same problem. However, they > have been borrowed from existing code from opcodes/cr16-dis.c > May I leave them here as is for now as it is used at several places in cr16-dis.c? Yes. I thought it was a variable declared inside GDB. This is fine. > 2012-11-20 Kaushik Phatak > > gdb/Changelog > * configure.tgt: Handle cr16*-*-*linux and cr16*-*-*. > * cr16-linux-tdep.c: New file. > * cr16-tdep.c: New file. > * cr16-tdep.h: New file. A couple of very very minor nits. Pre-approved with the changes requested below. Once you get approval for the BFD bits, you may commit this patch. When you do, can you send us a notification, as well as a copy of the patch that you commit (because it is going to be different from the one posted here). > + if (p->has_frame_ptr) > + { > + CORE_ADDR fp = get_frame_register_unsigned (this_frame, CR16_FP_REGNUM); > + return fp - p->frame_ptr_offset; > + } > + else > + { > + CORE_ADDR sp = get_frame_register_unsigned (this_frame, CR16_SP_REGNUM); > + return sp - p->frame_size; > + } Missing empty line after variable declaration in both blocks. > +}; Can you remov the trailing spaces? Thank you, -- Joel