From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-113736-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 8306 invoked by alias); 23 Jun 2014 13:18:14 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Received: (qmail 8282 invoked by uid 89); 23 Jun 2014 13:18:12 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2
X-HELO: mx1.redhat.com
Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 23 Jun 2014 13:18:09 +0000
Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5NDI5iE032103	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);	Mon, 23 Jun 2014 09:18:05 -0400
Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11])	by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5NDI2X3023094;	Mon, 23 Jun 2014 09:18:03 -0400
Message-ID: <53A8290A.1050701@redhat.com>
Date: Mon, 23 Jun 2014 13:18:00 -0000
From: Pedro Alves <palves@redhat.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: Ajit Kumar Agarwal <ajit.kumar.agarwal@xilinx.com>
CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,        Michael Eager <eager@eagercon.com>, Vinod Kathail <vinodk@xilinx.com>,        Vidhumouli Hunsigida <vidhum@xilinx.com>,        Nagaraju Mekala <nmekala@xilinx.com>
Subject: Re: [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal.
References: <fe2ea02f-37b9-4ebe-acd4-6d1192c26358@BN1AFFO11FD059.protection.gbl> <53A023B1.5000105@redhat.com> <859f27cb-8c46-46c1-9625-7287c60f3ae9@BY2FFO11FD007.protection.gbl> <53A1ABF0.9080004@redhat.com> <74281fd5-518a-4d7f-977a-6fa1320f6db9@BY2FFO11FD016.protection.gbl> <53A1B61F.9080803@redhat.com> <736c2e0d-6ff1-40c3-8120-dc6f5d91e6b1@BL2FFO11FD052.protection.gbl>
In-Reply-To: <736c2e0d-6ff1-40c3-8120-dc6f5d91e6b1@BL2FFO11FD052.protection.gbl>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
X-SW-Source: 2014-06/txt/msg00808.txt.bz2

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