Re: [Qemu-devel] [PATCH 2/3] use dr7's bit name for breakpoint

[Peter Maydell <peter.maydell@linaro.org>]

On 29 November 2012 03:32, liguang <lig.fnst@cn.fujitsu.com> wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  target-i386/cpu.h         |    2 ++
>  target-i386/helper.c      |   24 +++++++++++-------------
>  target-i386/misc_helper.c |    6 +++---
>  target-i386/seg_helper.c  |    6 +++---
>  4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 7f292e6..7ecfe21 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -561,6 +561,8 @@
>  /* dr7 fields */
>  /* max breakpoints*/
>  #define MAX_BP      4
> +/*enable local breakpoint bit 0,2,4,6*/
> +#define BP_LOCAL    0x55

This needs a better name, to make it clear that it's not
just a single enable bit but actually a mask of all the
local enable bits. Also needs DR7_ prefix.

You've split these changes up between patches inconsistently;
either have one patch which adds all the constants and
then patches which just use them, or have patches which
both add and use the constants, but don't mix the two.

I'd recommend that each patch should both add and use a
related set of constants, so it's self contained and
easy to review.

-- PMM