Re: [Qemu-devel] [PATCH 3/3] target-i386:refactor check_hw_breakpoints function

[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/helper.c |   28 ++++++++++++++++++++--------
>  1 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 9ca52a7..a506df0 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1012,22 +1012,34 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
>  int check_hw_breakpoints(CPUX86State *env, int force_dr6_update)
>  {
>      target_ulong dr6;
> -    int reg, type;
> +    int index;
>      int hit_enabled = 0;
>
>      dr6 = env->dr[6] & ~0xf;
> -    for (reg = 0; reg < MAX_BP; reg++) {
> -        type = hw_breakpoint_type(env->dr[7], reg);
> -        if ((type == 0 && env->dr[reg] == env->eip) ||
> -            ((type & 1) && env->cpu_watchpoint[reg] &&
> -             (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
> -            dr6 |= 1 << reg;
> -            if (hw_breakpoint_enabled(env->dr[7], reg))
> +    for (index = 0; index < MAX_BP; index++) {
> +        switch (hw_breakpoint_type(env->dr[7], index)){
> +        case BP_INST:
> +            if (env->dr[index] != env->eip)
> +                break;
> +            goto enable_hit;
> +        case BP_DATA_WR:
> +        case BP_DATA_RW:
> +            if (!env->cpu_watchpoint[index])
> +                break;
> +            if (!(env->cpu_watchpoint[index]->flags & BP_WATCHPOINT_HIT))
> +                break;
> +        enable_hit:
> +            dr6 |= 1 << index;
> +            if (hw_breakpoint_enabled(env->dr[7], index))
>                  hit_enabled = 1;
> +            break;
> +        case BP_IO_RW:
> +            break;
>          }

If you have to resort to gotos and fallthroughs in
a switch statement, I don't think this is actually
clearer than the existing code...

(Also it has coding style issues, use scripts/checkpatch.pl.)

-- PMM