Date: Fri, 15 Jan 2016 04:34:51 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r293979 - head/lib/libkvm Message-ID: <20160115031903.W9581@besplex.bde.org> In-Reply-To: <201601141551.u0EFpDTf052097@repo.freebsd.org> References: <201601141551.u0EFpDTf052097@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160115031903.W9581>