Re: [Patch, Microblaze]: Added support of shr and slr regs, little endian breakpoints,backtrace support and communicate with larger blocks.

[Joel Brobecker <brobecker at adacore dot com>]

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 <ajitkum@xilinx.com>
> 
>  * 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