Re: [Patch, microblaze]: Added cleanup data for invalid target description

[Pedro Alves <palves@redhat.com>]

On 10/07/2014 05:36 PM, Ajit Kumar Agarwal wrote:
> 
> 
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com] 
> Sent: Tuesday, October 07, 2014 9:37 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Added cleanup data for invalid target description
> 
> On 10/07/2014 11:16 AM, Ajit Kumar Agarwal wrote:
>>
>> From 00f2692d10e0254366471095516d657693aeff42 Mon Sep 17 00:00:00 2001
>> From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
>> Date: Tue, 7 Oct 2014 15:06:08 +0530
>> Subject: [PATCH] [Patch, microblaze]: Added cleanup data for invalid target description.
> 
>>> s/Added/Add/.  But even better would be saying what this actually intends to do, which is "reject".  Note the [PATCH] tag usually end ups stripped when >>the commit is imported into git, but the redundant [Patch, ...] seems like something you added manually, and is unnecessary.
> 
> Thanks for the suggestion.
>>
>> Cleanup the tdesc data if the target description check is invalid.
>>
>> 2014-10-07  Ajit Agarwal  <ajitkum@xilinx.com>
>>
>> 	* microblaze-tdep.c (microblaze_gdbarch_init): Use of
>> 	tdesc_data_cleanup.
> 
> So, I'd write:
> 
> ~~~
>>> [PATCH] Microblaze: Reject invalid target descriptions
> 
>>> We currently validate the target description, but then forget to reject it if found invalid.
> 
>>> gdb/
>>> 2014-10-07  Ajit Agarwal  <ajitkum@xilinx.com>
> 
>>> 	* microblaze-tdep.c (microblaze_gdbarch_init): If the description
>>> 	isn't valid, release the tdesc arch data and return NULL.
> ~~~
> 
> I will make this Change. Thanks for suggestion.
> 
>>> But, you didn't state how you tested this, which should be part of the commit log too.
> 
> I have tested the Microblaze Design with and without stack -protect registers. The gdb command "info registers" displayed the register correctly. If stack protect designs is not selected only core registers are displayed. When the stack-protect register is selected in the design, the core registers along with stack-protect registers are displayed.

Please remember to always state this in the commit/submit log.  I've
now added an explicit bullet item for that to the contribution
checklist.

> 
>>> Did you make sure incorrect descriptions are rejected and GDB warns about them?
> 
> We don't have the invalid Microblaze design with which I can test gdb warning and cleaning up the tdesc data.

Sure you do.  Just hack your target or gdbserver to send a bogus description.

Thanks,
Pedro Alves