[pushed] [PATCH V4] Add support for bound table in the Intel MPX context.

["Tedeschi, Walfred" <walfred dot tedeschi at intel dot com>]

Joel,

I followed your suggestion and pushed as considering that the rest was ok.
Hope this is fine.


Joel and Eli,

Thanks a lot for your review and valuable feedback!

Best regards,
-Fred

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
Sent: Tuesday, June 09, 2015 6:26 PM
To: Tedeschi, Walfred
Cc: eliz@gnu.org; gdb-patches@sourceware.org
Subject: Re: [PATCH V4] Add support for bound table in the Intel MPX context.

> 2015-04-20  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>             Mircea Gherzan  <mircea.gherzan@intel.com>
> 
> 	* i386-tdep.c (MPX_BASE_MASK, MPX_BD_MASK, MPX_BT_MASK, MPX_BD_MASK_32,
> 	MPX_BT_MASK_32): New macros.
> 	(i386_mpx_set_bounds): New function that implements
> 	the command "set-mpx-bound".
> 	(i386_mpx_enabled) Helper function to test MPX availability.
> 	(i386_mpx_bd_base) Helper function to calculate the base directory
> 	address. (i386_mpx_get_bt_entry) Helper function to access a bound
> 	table entry. (i386_mpx_print_bounds) Effectively display bound
> 	information. (_initialize_i386_tdep): Qdd new commands
> 	to commands "set mpx" and "show mpx". (_initialize_i386_tdep):
> 	Add "bound" to the commands "show mpx" and "set mpx" commands.
> 	(mpx_set_cmdlist and mpx_show_cmdlist):
> 	list for the new prefixed "set mpx" and "show mpx" commands.
> 	* NEWS: List new commands for MPX support.
> 
> testsuite:
> 
> 	* gdb.arch/i386-mpx-map.c: New file.
> 	* gdb.arch/i386-mpx-map.exp: New File.
> 
> doc:
> 	* gdb.texinfo (i386): Add documentation about "show mpx bound"
> 	and "set mpx bound".

> +/* Print routine for the mpx bounds.  */
> +
> +static void
> +i386_mpx_print_bounds (const CORE_ADDR bt_entry[])

I don't think we've been using the [] syntax for parameters, but I don't see a problem with that per se.

What I'm wondering, however, is the size of the array. In particular, you'll need, at least, to update the documentation. But if the size is actually statically known (which it seems is at most 4 elements), then perhaps it might be worth just saying it in the parameter type declaration...

Other than that, the patch looks good to me.



--
Joel
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052