From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19074 invoked by alias); 9 Oct 2014 23:42:19 -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 19062 invoked by uid 89); 9 Oct 2014 23:42:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: homiemail-a45.g.dreamhost.com Received: from sub5.mail.dreamhost.com (HELO homiemail-a45.g.dreamhost.com) (208.113.200.129) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 09 Oct 2014 23:42:17 +0000 Received: from homiemail-a45.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a45.g.dreamhost.com (Postfix) with ESMTP id DF5AD480B7; Thu, 9 Oct 2014 16:42:15 -0700 (PDT) Received: from redwood.eagercon.com (c-24-7-16-38.hsd1.ca.comcast.net [24.7.16.38]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: eager@eagerm.com) by homiemail-a45.g.dreamhost.com (Postfix) with ESMTPSA id 85FB3480C1; Thu, 9 Oct 2014 16:42:15 -0700 (PDT) Message-ID: <54371D57.2030605@eagerm.com> Date: Thu, 09 Oct 2014 23:42:00 -0000 From: Michael Eager User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Ajit Kumar Agarwal , Michael Eager , Pedro Alves , Joel Brobecker CC: "gdb-patches@sourceware.org" , Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: Re: [Patch] Microblaze: Port of Linux gdbserver References: <25de23b98e054fd291ea232d10f2800c@BN1BFFO11FD018.protection.gbl> <5436B7D0.9060004@eagercon.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg00228.txt.bz2 On 10/09/14 11:54, Ajit Kumar Agarwal wrote: > > To send the patches after incorporating the comments, Is there any other way of sending the patches without top post? After you address comments, include the changelog at the end and attach the patch (unless it is just a few lines). That way we can tell how you responded to each comment. > +#define microblaze_breakpoint_len 4 > >>> Use CAPS for macros. > > The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined. Let's follow the GNU coding standard, even if some other targets haven't. > + (*the_target->read_memory) (where, (unsigned char *) &insn, 4); > + > + if (insn == microblaze_breakpoint) > >>> Why use the explicit length rather than the macro you just defined? >>> Why not use sizeof (insn)? > > To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips 4 is used and in the ARM target for the THUMB_ARM 2 is used and for the ARM Mode code 4 is used. Let's follow good coding practice, even if there have been lapses in the past. Unless there is some particular relevance to instruction length on MIPS or ARM/Thumb, let's stick to what is relevant to MicroBlaze. > Pedro: >> I'd much prefer if we had that patch in the tree before accepting further patches that tweak things around register names, etc. Could you send that (as an independent patch, in a new thread). > >>> Please address issues with previous patches before moving on to submit dependent patches. > > I have already send the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments. I haven't seen this patch. Please let me know when you posted it, or send me a link to it in the mailing list archive. If you submit a patch which depends on previously submitted patches which have not been accepted, the new patch will not be accepted. Please don't submit dependent patches until all prior prerequisite patches are accepted. > Pedro: >> diff --git a/gdb/regformats/microblaze-with-stack-protect.dat > ... >> Please send a preparatory, independent, patch that updates >> features/Makefile instead and generates this file, in a new thread, >> with self-contained description, following the >> checklist: >> https://sourceware.org/gdb/wiki/ContributionChecklist > >>> Preparatory means that the patch should be submitted before the current patch. > > I will be sending this patch soon. OK. As mentioned above, please do not resubmit this patch until that patch is submitted and accepted. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077