From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=41393 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OsD8Y-00077c-Io
	for qemu-devel@nongnu.org; Sun, 05 Sep 2010 07:11:55 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <mst@redhat.com>) id 1OsA8v-0005HQ-La
	for qemu-devel@nongnu.org; Sun, 05 Sep 2010 04:00:02 -0400
Received: from mx1.redhat.com ([209.132.183.28]:19020)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <mst@redhat.com>) id 1OsA8v-0005HG-DT
	for qemu-devel@nongnu.org; Sun, 05 Sep 2010 04:00:01 -0400
Date: Sun, 5 Sep 2010 10:54:04 +0300
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20100905075403.GC16215@redhat.com>
References: <AANLkTimrAJ7fJzOhJRXLVpRjc8F+CoxWsETHss6tmZG3@mail.gmail.com>
	<AANLkTi=FSEZTUKQ38hgRXpxhjvYJW4wW3eSiy-aZy-AZ@mail.gmail.com>
	<AANLkTikGjBQ+T8XUhSj=HbKmJbx0H72JT9V5JEk2mRb3@mail.gmail.com>
	<AANLkTimosQmj8YZf2UrRqux8GwdmcMBp2Rz2aGVXZVO4@mail.gmail.com>
	<AANLkTinPwd6VrEdp-G4Vx5adsPPUe+-LG9svcU4RW_S+@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <AANLkTinPwd6VrEdp-G4Vx5adsPPUe+-LG9svcU4RW_S+@mail.gmail.com>
Content-Transfer-Encoding: quoted-printable
Subject: [Qemu-devel] Re: [PATCH 1/5] Suppress some gcc warnings with
	-Wtype-limits
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-devel <qemu-devel@nongnu.org>

On Sat, Sep 04, 2010 at 05:21:24PM +0000, Blue Swirl wrote:
> In the unsigned number space, the checks can be merged into one,
> assuming that BLKDBG_EVEN_MAX is less than INT_MAX. Alternatively we
> could have:
>  -    if (event < 0 || event >=3D BLKDBG_EVENT_MAX) {
>  +    if ((int)event < 0 || event >=3D BLKDBG_EVENT_MAX) {
>=20
> This would also implement the check that the writer of this code was
> trying to make.
> The important thing to note is however is that the check as it is now
> is not correct.

I agree. But it seems to indicate a bigger problem.

If we are trying to pass in a negative value, which is not one
of enum values, using BlkDebugEvent as type is just confusing,
we should just pass int instead.


> >> How about adding assert(OMAP_EMIFS_BASE =3D=3D 0) and commenting out=
 the
> >> check? Then if the value changes, the need to add the comparison bac=
k
> >> will be obvious.
> >
> > This would work but it's weird. =A0The thing is it's currently a corr=
ect
> > code and the check may be useless but it's the optimiser's task to
> > remove it, not ours. =A0The compiler is not able to tell whether the
> > check makes sense or nott, because the compiler only has access to
> > preprocessed code. =A0So why should you let the compiler have anythin=
g
> > to say on it.
>=20
> Good point. I'll try to invent something better.

Use #pragma to supress the warning? Maybe we could wrap this in a macro ..

--=20
MST