From owner-svn-src-head@freebsd.org Thu Jan 14 17:35:01 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C942FA82E39; Thu, 14 Jan 2016 17:35:01 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 91D401381; Thu, 14 Jan 2016 17:35:01 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 8B2CD78133F; Fri, 15 Jan 2016 04:34:52 +1100 (AEDT) Date: Fri, 15 Jan 2016 04:34:51 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John Baldwin cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r293979 - head/lib/libkvm In-Reply-To: <201601141551.u0EFpDTf052097@repo.freebsd.org> Message-ID: <20160115031903.W9581@besplex.bde.org> References: <201601141551.u0EFpDTf052097@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=cK4dyQqN c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=JzwRw_2MAAAA:8 a=kj9zAlcOel0A:10 a=IwOPfhtQleq6pfVgbWMA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jan 2016 17:35:01 -0000 On Thu, 14 Jan 2016, John Baldwin wrote: > Log: > Fix building with GCC since PAGE_MASK is signed on i386. > ... > Modified: head/lib/libkvm/kvm_i386.h > ============================================================================== > --- head/lib/libkvm/kvm_i386.h Thu Jan 14 15:50:13 2016 (r293978) > +++ head/lib/libkvm/kvm_i386.h Thu Jan 14 15:51:13 2016 (r293979) > @@ -70,7 +70,7 @@ _Static_assert(NBPDR == I386_NBPDR, "NBP > > _Static_assert(PG_V == I386_PG_V, "PG_V mismatch"); > _Static_assert(PG_PS == I386_PG_PS, "PG_PS mismatch"); > -_Static_assert(PG_FRAME == I386_PG_FRAME, "PG_FRAME mismatch"); > +_Static_assert((u_int)PG_FRAME == I386_PG_FRAME, "PG_FRAME mismatch"); > _Static_assert(PG_PS_FRAME == I386_PG_PS_FRAME, "PG_PS_FRAME mismatch"); > #endif This breaks the warning. PG_FRAME still has the fragile type int and the fragile value -4096. libkvm has lots of nearby style bugs like using the long long abomination as a suffix for the literal constants, and bogus parentheses around single literals. All of these are copied from the kernel (except their ames are changed by adding an I386_ prefix). The assertions aren't very useful when half of the constants use lexically identically definitions with equal ugliness. The 2 PAE constants are the main ones that aren't asserted to be equal. I think PAE in the kernel could even use the signedness of PG_FRAME. (vm_paddr_t)-4096 first sign extends to (int64_t) with value -4096 and then overflows to the correct mask of 0xfffffffffffff000 with the correct type. So -4096 could work for both PAE and !PAE. But PAE actually uses an ifdef to to define PG_FRAME as (0x000ffffffffff000ull). This has the 2 style bugs mentioned above, and seems to have a nonsense value. I think PAE is only 36 bits, but the mask is for 52 bits. I don't like explicitly typed constants, but this problem shows that some care is needed for using constants in expressions. int is correct for PAGE_SIZE but not masks, except for masks of all except the sign bit it is OK. Bruce