From owner-svn-src-all@FreeBSD.ORG Thu Jun 16 18:03:51 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 5D96F1065672; Thu, 16 Jun 2011 18:03:51 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from mail.icecube.wisc.edu (trout.icecube.wisc.edu [128.104.255.119]) by mx1.freebsd.org (Postfix) with ESMTP id 1B8618FC13; Thu, 16 Jun 2011 18:03:50 +0000 (UTC) Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.icecube.wisc.edu (Postfix) with ESMTP id 360E058140; Thu, 16 Jun 2011 13:03:50 -0500 (CDT) X-Virus-Scanned: amavisd-new at icecube.wisc.edu Received: from mail.icecube.wisc.edu ([127.0.0.1]) by localhost (trout.icecube.wisc.edu [127.0.0.1]) (amavisd-new, port 10030) with ESMTP id SyRDx9BbvLjd; Thu, 16 Jun 2011 13:03:50 -0500 (CDT) Received: from wanderer.tachypleus.net (i3-dhcp-172-16-223-128.icecube.wisc.edu [172.16.223.128]) by mail.icecube.wisc.edu (Postfix) with ESMTP id E024A5811D; Thu, 16 Jun 2011 13:03:49 -0500 (CDT) Message-ID: <4DFA4585.4040500@freebsd.org> Date: Thu, 16 Jun 2011 13:03:49 -0500 From: Nathan Whitehorn User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.17) Gecko/20110516 Thunderbird/3.1.10 MIME-Version: 1.0 To: Garrett Cooper References: <201106160714.p5G7Etfx017112@svn.freebsd.org> <20110616180803.D1005@besplex.bde.org> <11061619555315.44181@www.mmlab.cse.yzu.edu.tw> <20110616235239.D1926@besplex.bde.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Tai-hwa Liang , src-committers@freebsd.org, Bruce Evans Subject: Re: svn commit: r223139 - head/lib/libstand 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: Thu, 16 Jun 2011 18:03:51 -0000 On 06/16/11 10:23, Garrett Cooper wrote: > On Thu, Jun 16, 2011 at 8:12 AM, Garrett Cooper wrote: >> On Thu, Jun 16, 2011 at 7:06 AM, Bruce Evans wrote: >>> On Thu, 16 Jun 2011, Tai-hwa Liang wrote: >>> >>>> On Thu, 16 Jun 2011, Bruce Evans wrote: >>>> >>>>> On Thu, 16 Jun 2011, Garrett Cooper wrote: >>>>>> >>>>>> And you need to add #include to stand.h in order to get >>>>>> uintmax_t. Here's a proper patch for amd64.. >>>>> >>>>> This would add namespace pollution. stand.h doesn't use anything in >>>>> . It depends on normal namespace pollution in an XXX section >>>>> in for the declaration of uintptr_t. It and other headers >>>>> should use __uintptr_t instead. Strangely, declares >>>>> uintptr_t but not uintmax_t. >>>> >>>> What about casting to __uintmax_t instead? >>>> >>>> Index: zalloc.c >>>> =================================================================== >>>> --- zalloc.c (revision 223146) >>>> +++ zalloc.c (working copy) >>>> @@ -154,7 +154,7 @@ >>>> if ((char *)ptr< (char *)mp->mp_Base || >>>> (char *)ptr + bytes> (char *)mp->mp_End || >>>> ((iaddr_t)ptr& MEMNODE_SIZE_MASK) != 0) >>>> - panic("zfree(%p,%ju): wild pointer", ptr, bytes); >>>> + panic("zfree(%p,%ju): wild pointer", ptr, (__uintmax_t)bytes); >>>> ... >>> >>> zalloc.c is not the (header) implementation, so it should not use the >>> implementation detail (anything beginning with __). >>> >>> The latest tinderbox errors for this are hard to understand. For amd64 >>> they say: >>> >>>> /src/lib/libstand/zalloc.c: In function 'zfree': >>>> /src/lib/libstand/zalloc.c:157: warning: format '%ju' expects type >>>> 'uintmax_t', but argument 3 has type 'iaddr_t' >>> >>> but amd64 seems to be just like sparc64 -- both seem to declare all the >>> types as `unsigned long' at the lowest level. I would expect all 64-bit >>> arches to do this, although this is logically wrong (it makes the largest >>> integral type uintmax_t logically smaller than the standard bogus type >>> unsigned long long). This logic error is partly intentional (it detects >>> different type mismatches than uintmax_t = `unsigned long long' combined >>> with uint64_t = `unsigned long' would). >> >> My second patch when applied gets one past tinderbox on powerpc.. >> waiting for powerpc64 and sparc64 to complete. Namespace pollution is >> one thing, but stdint.h isn't that bad IMHO -- I just find it funny >> that iaddr_t had to be typedef'ed to uintptr_t in the first place. > > This needs to be committed to unbreak ia64 and mips64*. Still waiting > to see what happens with sparc64, but amd64 and powerpc64 were oddly > happy without this, even though they should have failed along with > ia64 and mips64* (the format string should have been %zu for a size_t > type). Both amd64 and powerpc64 use 32-bit bootloaders, and so libstand is built with a 32-bit toolchain. As such, bugs affecting 64-bit platforms don't show up for libstand builds on these systems. -Nathan