From owner-svn-src-all@FreeBSD.ORG Tue Jan 18 18:42:41 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 31CC7106564A; Tue, 18 Jan 2011 18:42:41 +0000 (UTC) (envelope-from rdivacky@vlk.vlakno.cz) Received: from vlakno.cz (lev.vlakno.cz [77.93.215.190]) by mx1.freebsd.org (Postfix) with ESMTP id B1CD68FC13; Tue, 18 Jan 2011 18:42:40 +0000 (UTC) Received: from lev.vlakno.cz (localhost [127.0.0.1]) by vlakno.cz (Postfix) with ESMTP id D729F9CB0F4; Tue, 18 Jan 2011 19:26:56 +0100 (CET) X-Virus-Scanned: amavisd-new at vlakno.cz Received: from vlakno.cz ([127.0.0.1]) by lev.vlakno.cz (lev.vlakno.cz [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pIRWgWUTMe6Q; Tue, 18 Jan 2011 19:26:56 +0100 (CET) Received: from vlk.vlakno.cz (localhost [127.0.0.1]) by vlakno.cz (Postfix) with ESMTP id 2CA189CB158; Tue, 18 Jan 2011 19:26:56 +0100 (CET) Received: (from rdivacky@localhost) by vlk.vlakno.cz (8.14.4/8.14.4/Submit) id p0IIQtXO079930; Tue, 18 Jan 2011 19:26:55 +0100 (CET) (envelope-from rdivacky) Date: Tue, 18 Jan 2011 19:26:55 +0100 From: Roman Divacky To: Bruce Evans Message-ID: <20110118182655.GA77102@freebsd.org> References: <201101181523.p0IFNGeB042079@svn.freebsd.org> <20110119035030.W2099@besplex.bde.org> <201101181215.46266.jhb@freebsd.org> <20110119044805.Y2631@besplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110119044805.Y2631@besplex.bde.org> User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin Subject: Re: svn commit: r217538 - in head/sys/dev: buslogic cs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Jan 2011 18:42:41 -0000 On Wed, Jan 19, 2011 at 04:54:34AM +1100, Bruce Evans wrote: > On Tue, 18 Jan 2011, John Baldwin wrote: > > >On Tuesday, January 18, 2011 12:00:44 pm Bruce Evans wrote: > >>On Tue, 18 Jan 2011, John Baldwin wrote: > >> > >>>Log: > >>> Remove some always-true comparisons. > >>> > >>> Submitted by: clang via rdivacky > >>> > >>>Modified: head/sys/dev/buslogic/bt.c > >>> > >============================================================================== > >>>--- head/sys/dev/buslogic/bt.c Tue Jan 18 14:58:44 2011 (r217537) > >>>+++ head/sys/dev/buslogic/bt.c Tue Jan 18 15:23:16 2011 (r217538) > >>>@@ -975,7 +975,7 @@ bt_find_probe_range(int ioport, int *por > >>>int > >>>bt_iop_from_bio(isa_compat_io_t bio_index) > >>>{ > >>>- if (bio_index >= 0 && bio_index < BT_NUM_ISAPORTS) > >>>+ if (bio_index < BT_NUM_ISAPORTS) > >>> return (bt_board_ports[bio_index]); > >>> return (-1); > >>>} > >> > >>So, what guarantees that isa_compat_io_t is unsigned and will remain so? > >>Indexes should be ints, unless you want a sign morass. > > > >Gah, I trusted the clang warning too much. enum's are ints in C yes? So > >clang has a bug if it thinks an enum value cannot be negative. In practice > >all the callers of this routine do not pass in negative values, but the > >compiler shouldn't warn about enum's bogusly. > > I didn't know it was a problem already when I asked. I thought > isa_compat_io_t was global, but it is private to bt. > > >Is clang assuming that only defined values for an enum are ever passed in? > >If > >so we probably don't want to turn that checking on for the kernel as we > >violate that in lots of places. > > enum values are int, but an enum type may be implemented as unsigned if > that makes no difference for conforming code. I think conforming code can > only used declared enum values. Thus if all declared values are >= 0, as > is the case here, then the enum type may be implemented as unsigned. > Apparently, this happens for clang here, and it checks what it would do > itself. Other compilers might do it differently. No, it's a real bug - C99 says that: If an int can represent all values of the original type, the value is converted to an int; otherwise, it is converted to an unsigned int. " (C99 6.3.1.1p2). Thus in this case the enum must be (signed) int. The problem is that this bug is in gcc too, compare: witten ~# cat enum1.c #include static enum { foo, bar = 1U } z; int main (void) { int r = 0; if (z - 1 < 0) r++; printf("r = %i\n", r); } witten ~# gcc enum1.c && ./a.out r = 0 witten ~# g++ enum1.c && ./a.out r = 1 So clang is just emulating gcc bug which results in this unfortunate bogus warning.