Patchwork [3/3] target-i386:refactor check_hw_breakpoints function

login
register
mail settings
Submitter liguang
Date Nov. 29, 2012, 3:32 a.m.
Message ID <1354159971-6720-3-git-send-email-lig.fnst@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/202652/
State New
Headers show

Comments

liguang - Nov. 29, 2012, 3:32 a.m.
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 target-i386/helper.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)
Peter Maydell - Nov. 29, 2012, 11:31 a.m.
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

Patch

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 (hit_enabled || force_dr6_update)
         env->dr[6] = dr6;
+
     return hit_enabled;
 }