Re: [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-12-05 01:51, li guang wrote:
> 在 2012-12-04二的 11:26 +0000，Peter Maydell写道：
>> On 4 December 2012 11:11, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2012-12-04 11:23, Peter Maydell wrote:
>>>> Doesn't this break the use of this function in target-i386/seg_helper.c:
>>>>
>>>>   if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
>>>>
>>>> which specifically wants to determine whether the breakpoint is
>>>> enabled only locally?
> 
>  It was changed to 'if (hw_breakpoint_enabled(env->dr[7], i)) {'
>  in patch 3/3

Which is broken as it neglects the different types of "enabled".

> 
>>>
>>> It does. And that also indicates the function is misnamed. Something
>>> like hw_breakpoint_state might be better.
>>
> 
> misnamed? I think hw_breakpoint_enabled is ask whether breakpoint
>                                  ^^^^^^^^
> is enabled or not, so it's almost suitable.

There are two types of enabled breakpoints: task-local and global. The
current hw_breakpoint_enabled returns both as a bitmask, and that is
causing the confusing and regression in your patches.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux