Date: Thu, 16 Jun 2011 13:03:49 -0500 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Garrett Cooper <yanegomi@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Tai-hwa Liang <avatar@mmlab.cse.yzu.edu.tw>, src-committers@freebsd.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r223139 - head/lib/libstand Message-ID: <4DFA4585.4040500@freebsd.org> In-Reply-To: <BANLkTinNn_74N4Zp%2BjWPq%2Bh9ue6uDMMyRg@mail.gmail.com> References: <201106160714.p5G7Etfx017112@svn.freebsd.org> <BANLkTi=X0_SBLAQ6t7amTLv7jF6_oXAV4Q@mail.gmail.com> <BANLkTimG4svFzv1QPiKQcC7QdChLica9xA@mail.gmail.com> <20110616180803.D1005@besplex.bde.org> <11061619555315.44181@www.mmlab.cse.yzu.edu.tw> <20110616235239.D1926@besplex.bde.org> <BANLkTinpp95JyEp8sVR7uL-sAWor-69mCA@mail.gmail.com> <BANLkTinNn_74N4Zp%2BjWPq%2Bh9ue6uDMMyRg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 06/16/11 10:23, Garrett Cooper wrote:
> On Thu, Jun 16, 2011 at 8:12 AM, Garrett Cooper<yanegomi@gmail.com> wrote:
>> On Thu, Jun 16, 2011 at 7:06 AM, Bruce Evans<brde@optusnet.com.au> 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<stdint.h> 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
>>>>> <stdint.h>. It depends on normal namespace pollution in an XXX section
>>>>> in<sys/types.h> for the declaration of uintptr_t. It and other headers
>>>>> should use __uintptr_t instead. Strangely,<sys/types.h> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DFA4585.4040500>