From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50292) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgAkB-0007k4-Su for qemu-devel@nongnu.org; Wed, 05 Dec 2012 03:54:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TgAk5-0005iy-VO for qemu-devel@nongnu.org; Wed, 05 Dec 2012 03:54:15 -0500 Received: from thoth.sbs.de ([192.35.17.2]:23457) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TgAk5-0005ii-Ls for qemu-devel@nongnu.org; Wed, 05 Dec 2012 03:54:09 -0500 Message-ID: <50BF0BA3.6080906@siemens.com> Date: Wed, 05 Dec 2012 09:53:55 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1354608695-3232-1-git-send-email-lig.fnst@cn.fujitsu.com> <1354608695-3232-2-git-send-email-lig.fnst@cn.fujitsu.com> <50BDDA54.9020000@siemens.com> <1354668685.4342.5.camel@liguang.fnst.cn.fujitsu.com> In-Reply-To: <1354668685.4342.5.camel@liguang.fnst.cn.fujitsu.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: li guang Cc: Peter Maydell , "ehabkost@redhat.com" , "qemu-devel@nongnu.org" , "blauwirbel@gmail.com" , "imammedo@redhat.com" , "afaerber@suse.de" On 2012-12-05 01:51, li guang wrote: > =E5=9C=A8 2012-12-04=E4=BA=8C=E7=9A=84 11:26 +0000=EF=BC=8CPeter Maydel= l=E5=86=99=E9=81=93=EF=BC=9A >> On 4 December 2012 11:11, Jan Kiszka wrote: >>> On 2012-12-04 11:23, Peter Maydell wrote: >>>> Doesn't this break the use of this function in target-i386/seg_helpe= r.c: >>>> >>>> if (hw_breakpoint_enabled(env->dr[7], i) =3D=3D 0x1) { >>>> >>>> which specifically wants to determine whether the breakpoint is >>>> enabled only locally? >=20 > 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". >=20 >>> >>> It does. And that also indicates the function is misnamed. Something >>> like hw_breakpoint_state might be better. >> >=20 > 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 --=20 Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux