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

[Pedro Alves <palves@redhat.com>]

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.

> 
> 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.
~~~

But, you didn't state how you tested this, which should be part
of the commit log too.

Did you make sure incorrect descriptions are rejected and GDB warns
about them?

Did you make sure valid descriptions do end up correctly used?

Or does this uncover other bugs?

Thanks,
Pedro Alves