public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Walfred Tedeschi <walfred.tedeschi@intel.com>
Cc: eliz@gnu.org, gdb-patches@sourceware.org
Subject: Re: [PATCH V4] Add support for bound table in the Intel MPX context.
Date: Tue, 09 Jun 2015 16:26:00 -0000	[thread overview]
Message-ID: <20150609162607.GA2855@adacore.com> (raw)
In-Reply-To: <1433856847-16851-1-git-send-email-walfred.tedeschi@intel.com>

> 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

      parent reply	other threads:[~2015-06-09 16:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-09 13:34 Walfred Tedeschi
2015-06-09 14:33 ` Eli Zaretskii
2015-06-09 14:56   ` Tedeschi, Walfred
2015-06-09 16:26 ` Joel Brobecker [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150609162607.GA2855@adacore.com \
    --to=brobecker@adacore.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=walfred.tedeschi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).