From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51222 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OsMQG-0000Op-DN for qemu-devel@nongnu.org; Sun, 05 Sep 2010 17:06:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OsMQ9-0001yU-6u for qemu-devel@nongnu.org; Sun, 05 Sep 2010 17:06:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5323) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OsMQ8-0001yD-UI for qemu-devel@nongnu.org; Sun, 05 Sep 2010 17:06:37 -0400 Date: Mon, 6 Sep 2010 00:00:36 +0300 From: "Michael S. Tsirkin" Message-ID: <20100905210035.GA5423@redhat.com> References: <20100905175735.GB24336@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH 08/15] blkdebug: fix enum comparison' List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel On Sun, Sep 05, 2010 at 07:37:54PM +0000, Blue Swirl wrote: > On Sun, Sep 5, 2010 at 5:57 PM, Michael S. Tsirkin wro= te: > > On Sun, Sep 05, 2010 at 03:06:32PM +0000, Blue Swirl wrote: > >> The signedness of enum types depend on the compiler implementation. > >> Therefore the check for negative values may or may not be meaningful. > >> > >> Fix by explicitly casting to a signed integer. > >> > >> Since the values are also checked earlier against event_names > >> table, this is an internal error. Change the 'if' to 'assert'. > >> > >> This also fixes a warning with GCC flag -Wtype-limits. > >> > >> Signed-off-by: Blue Swirl > >> --- > >> =A0block/blkdebug.c | =A0 =A04 +--- > >> =A01 files changed, 1 insertions(+), 3 deletions(-) > >> > >> diff --git a/block/blkdebug.c b/block/blkdebug.c > >> index 2a63df9..4d6ff0a 100644 > >> --- a/block/blkdebug.c > >> +++ b/block/blkdebug.c > >> @@ -439,9 +439,7 @@ static void blkdebug_debug_event(BlockDriverStat= e > >> *bs, BlkDebugEvent event) > >> =A0 =A0 =A0struct BlkdebugRule *rule; > >> =A0 =A0 =A0BlkdebugVars old_vars =3D s->vars; > >> > >> - =A0 =A0if (event < 0 || event >=3D BLKDBG_EVENT_MAX) { > >> - =A0 =A0 =A0 =A0return; > >> - =A0 =A0} > >> + =A0 =A0assert((int)event >=3D 0 && event < BLKDBG_EVENT_MAX); > > > > I am not sure all compilers must generate a negative value from > > a very large unsigned integer cast to int. >=20 > The enum rules seem to be vague. The type of enums may also be signed > (on GCC when the enum set includes negative values, on other compilers > in other cases). Do any machines or compilers exist (on which QEMU > runs) where this could happen? I remember reading that GCC sometimes assumes signed integers don't overf= low, and generates code behaves incorrectly if they do. No idea whether this is ever the case for casts. --=20 MST