From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21055 invoked by alias); 20 May 2014 13:13:40 -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 20877 invoked by uid 89); 20 May 2014 13:13:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 20 May 2014 13:13:36 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 9F7961160DB; Tue, 20 May 2014 09:13:34 -0400 (EDT) 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 B3SaZ+mYoujm; Tue, 20 May 2014 09:13:34 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 6DABB1160AA; Tue, 20 May 2014 09:13:34 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id EF60F43848; Tue, 20 May 2014 06:13:32 -0700 (PDT) Date: Tue, 20 May 2014 13:13:00 -0000 From: Joel Brobecker To: Ajit Kumar Agarwal Cc: "gdb-patches@sourceware.org" , Michael Eager , Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: Re: [Patch, Microblaze]: Added support of shr and slr regs, little endian breakpoints,backtrace support and communicate with larger blocks. Message-ID: <20140520131332.GF22822@adacore.com> References: <57db2c05-1b1a-472b-9182-5ce078e8805a@BY2FFO11FD055.protection.gbl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57db2c05-1b1a-472b-9182-5ce078e8805a@BY2FFO11FD055.protection.gbl> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-05/txt/msg00415.txt.bz2 Hello Ajit, > This patch added the support of shr and slr regs and little endian > breakpoints, backtrace support without debug information and > communicate larger blocks with debugging agent. Thanks for the patch. My first comment is that you need to split the patches into small individual patches instead of submitting everything in one bulk patch. This will facilitate review, thus increasing the chances of getting this patch in sooner rather than later. And it's also a good principle to follow in case we want to revert one part of the change. > > ChangeLog: > 2014-05-20 Ajit Agarwal > > * gdb/gdbserver/Makefile.in (microblaze-linux.c) : New target Not sure if this intended or not, but the entry should be indented with a tab. No space before the ':'. And all sentences should end with a period (as well as starting with a capital letter as you did here). > > * gdb/microblaze-tdep.c > (microblaze_register_names): Added the rshr and rslr register names. > (microblaze_breakpoint_from_pc): Added Declaration of byte_order The indentation for all lines should be one tab, so the subsequent lines should not be over indented. Please also join the first and second lines, as there is no need to skip to the next line here. > and break_insn_le. Check of byte order by BFD_ENDIAN_BIG > (microblaze_alloc_frame_cache): Initialize saved_sp > (microblaze_analyze_prologue): Do a block read to minimize the > transaction with debug agent. Use of target_read_memory. Freeing > the block read. > (microblaze_frame_cache): use of microblaze_analyze_prologue and > assigning the register_offsets. > (microblaze_frame_prev_register): Use of frame_unwind_got_constant. > Check of regnum with MICROBLAZE_SP_REGNUM ,MICROBLAZE_R19_REGNUM, > MICROBLAZE_PC_REGNUM. > > * gdb/microblaze-tdep.h > (microblaze_frame_cache): Addition of fields saved_sp,modification of > register_offsets. > (microblaze_reg_num): Addition of fields MICROBLAZE_SLR_REGNUM and > MICROBLAZE_SHR_REGNUM. > (MICROBLAZE_BREAKPOINT_LE): New Macro. > > * gdb/regformats/reg-microblaze.dat: New Register Data files for Microblaze. Can you also make sure that, once re-indented properly, none of the lines exceed 74 characters? It's a soft limit, so you can exceed it a bit if it helps, but the absolute hard limit is 80 characters. We'll try to review the patches themselves once they have been split. I hope I don't sound too hard. The GDB Coding Style, which is derived from the GNU Coding Style, can take a while to learn. -- Joel