qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 2/3] target-i386:make hw_breakpoint_enabled r


From: li guang
Subject: Re: [Qemu-devel] [PATCH v3 2/3] target-i386:make hw_breakpoint_enabled return bool type
Date: Mon, 10 Dec 2012 10:24:13 +0800

在 2012-12-07五的 11:29 +0100,Jan Kiszka写道:
> On 2012-12-07 11:24, Peter Maydell wrote:
> > On 7 December 2012 01:25, liguang <address@hidden> wrote:
> >> Signed-off-by: liguang <address@hidden>
> >> ---
> >>  target-i386/cpu.h |   15 +++++++++++++--
> >>  1 files changed, 13 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> >> index 29245d1..3646128 100644
> >> --- a/target-i386/cpu.h
> >> +++ b/target-i386/cpu.h
> >> @@ -996,9 +996,20 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, 
> >> target_ulong addr,
> >>  #define cpu_handle_mmu_fault cpu_x86_handle_mmu_fault
> >>  void cpu_x86_set_a20(CPUX86State *env, int a20_state);
> >>
> >> -static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
> >> +static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int 
> >> index)
> >>  {
> >> -    return (dr7 >> (index * 2)) & 3;
> >> +    return !(((dr7 >> (index * 2)) ^ 1) & 3);
> > 
> > This is pretty confusing and I'm pretty sure the function is
> > misnamed too. If we're checking "is local breakpoint enabled"
> > then we only want to check one of the two enable bits, not both.
> > 
> 
> Yes, and I already asked to define the proper constants that allow
> checking for local vs. global enable bit. They have to be used here
> instead of all the magic & 3 or ^ 1 stuff.
> 
> BTW, there is no need for "converting" ("!!") the result of the (value &
> mask) to bool, the compiler will do this already.
> 
> Jan

OK, as both of you commented, I'll fix for them.
Thanks!

> 
> > 
> >> +}
> >> +
> >> +static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int 
> >> index)
> >> +{
> >> +    return !!((dr7 >> (index * 2)) & 2);
> >> +}
> >> +
> >> +static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
> >> +{
> >> +    return (hw_global_breakpoint_enabled(dr7, index) ||
> >> +            hw_local_breakpoint_enabled(dr7, index));
> >>  }
> > 
> > -- PMM
> > 
> 

-- 
regards!
li guang




reply via email to

[Prev in Thread] Current Thread [Next in Thread]