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

Pedro Alves
2014-06-23T13:18:14Z

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