Re: [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal.

[Pedro Alves <palves at redhat dot com>]

On 06/18/2014 08:56 PM, Ajit Kumar Agarwal wrote:
> Based on feedback, review comments are incorporated. Could you please review
> and let me know if its okay.
> 
>     [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal.
>     
>     Prior to version MicroBlaze v8.10.a,EDK 13.1, XMD's gdbserver stub returned 57
>     registers in response to GDB's G request.  Starting with version MicroBlaze
>     v8.10.a, EDK 13.1, XMD added the slr and shr register, for a count of 59
>     registers.This patch adds these registers to the expected G response. This patch

(Please mind the the missing spaces after period in the commit log above.)

>     fixes the above problem for baremetal and also supports the backward compatibility.
>     
>     ChangeLog:
>     2014-06-19 Ajit Agarwal <ajitkum@xilinx.com>

Two spaces before and after name.

> +<!-- Copyright (C) 2007-2014 Free Software Foundation, Inc.

I assume this 2007 was just a copy/paste?  As this is all new content
only "2014" is appropriate.

> +++ b/gdb/features/microblaze-stack-protect.xml
> @@ -0,0 +1,12 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2009-2014 Free Software Foundation, Inc.

Likewise.


> +  if (tdesc_has_registers (tdesc))
> +    {
> +

Spurious empty line.

> +      const struct tdesc_feature *feature;


> +      int valid_p;
> +      int i;
> +      feature = tdesc_find_feature (tdesc,

Empty line after declarations missing.

> +                                    "org.gnu.gdb.microblaze.core");
> +      if (feature == NULL)
> +          return NULL;

Indentation of the return statement isn't right.

> +      tdesc_data = tdesc_data_alloc ();
> +
> +      valid_p = 1;
> +      for (i = 0; i < MICROBLAZE_NUM_CORE_REGS; i++)
> +        valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
> +                                            microblaze_register_names[i]);
> +      feature = tdesc_find_feature (tdesc,
> +                                    "org.gnu.gdb.microblaze.stack-protect");
> +
> +      if (feature != NULL)
> +        {
> +          valid_p = 1;
> +          valid_p &= tdesc_numbered_register (feature, tdesc_data,
> +                                              MICROBLAZE_SLR_REGNUM,
> +                                              "rslr");
> +          valid_p &= tdesc_numbered_register (feature, tdesc_data,
> +                                              MICROBLAZE_SHR_REGNUM,
> +                                              "rshr");
> +        }
> +     }
> +  tdep = xcalloc (1, sizeof (struct gdbarch_tdep));
> +  gdbarch = gdbarch_alloc (&info, tdep);
> +
> +  microblaze_register_g_packet_guesses (gdbarch);
> +
>  
>    /* Allocate space for the new architecture.  */
>    tdep = XNEW (struct gdbarch_tdep);
> @@ -725,7 +778,11 @@ microblaze_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    dwarf2_append_unwinders (gdbarch);
>    frame_unwind_append_unwinder (gdbarch, &microblaze_frame_unwind);
>    frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
> -
> +  if (tdesc_data)

Write:

  if (tdesc_data != NULL)

Otherwise I have no further comments.

Thanks,
-- 
Pedro Alves