Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 8 Sep 2014 14:19:44 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Rui Paulo <rpaulo@me.com>
Cc:        Davide Italiano <davide@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r270261 - head/sys/sys
Message-ID:  <20140908131951.D880@besplex.bde.org>
In-Reply-To: <0DD8929C-983D-49FD-A904-7A4FF9A3D2CB@me.com>
References:  <201408210901.s7L91gZ7049822@svn.freebsd.org> <0DD8929C-983D-49FD-A904-7A4FF9A3D2CB@me.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 7 Sep 2014, Rui Paulo wrote:

> On Aug 21, 2014, at 02:01, Davide Italiano <davide@freebsd.org> wrote:
>>
>> Author: davide
>> Date: Thu Aug 21 09:01:42 2014
>> New Revision: 270261
>> URL: http://svnweb.freebsd.org/changeset/base/270261
>>
>> Log:
>>  Revert r270227. GCC doesn't like the lack of LL suffix,
>>  so this makes powerpc build failing.
>>
>> Modified:
>>  head/sys/sys/time.h
>>
>> Modified: head/sys/sys/time.h
>> ==============================================================================
>> --- head/sys/sys/time.h	Thu Aug 21 08:25:46 2014	(r270260)
>> +++ head/sys/sys/time.h	Thu Aug 21 09:01:42 2014	(r270261)
>> @@ -129,7 +129,7 @@ bintime_shift(struct bintime *_bt, int _
>> #define	SBT_1MS	(SBT_1S / 1000)
>> #define	SBT_1US	(SBT_1S / 1000000)
>> #define	SBT_1NS	(SBT_1S / 1000000000)
>> -#define	SBT_MAX	0x7fffffffffffffff
>> +#define	SBT_MAX	0x7fffffffffffffffLL
>
> I also think this is more correct.

This is abominably less correct:
- it uses the long long abomination
- the long long abomination doesn't even give the correct type on any 64-bit
   arch, except possibly on broken development versions of arm or mips where
   the long long abomination is used for int64_t.  The correct type is
   sbintime_t = int64_t.  This is plain long on all 64-bit arches except
   possibly the broken ones.
- the unsuffixed constant automatically has the correct type (the smallest
   one that works), except possibly in preprocessor expressions where the
   rules are quite different
- the correct type is already used for all the other SBT constants visible
   in the diff.  Just 1 line outside of the diff there is an example of a
   correct definition:

% #define	SBT_1S	((sbintime_t)1 << 32)

   The cast in this makes it unsuitable for use in preprocessor expressions,
   but since that is apparently not a problem for any of the old definitions
   it should be even less of a problem for a new definition.  None of this
   is documented.  If it were documented, the documenation should say that
   none of these macros is suitable for use in a preprocessor expressions.

The abomination is being used to work around a compiler warning for broken
C90 compilers on 32-bit systems.  These compilers support long long and
don't warn for it, but they need to support large constants to go with
the long longs and they do warn for those unless they have an LL suffix.
The idea is to require all uses of long long to be explicit.  This may
have been better for 32-bit arches in 1990, but it was worse for 64-bit
arches even then (except broken ones that made long 32 bits).  These
arches never needed long long.

powerpc is apparently still using the 1990 compiler options that give this
warning.  That's in the kernel.  Compiler options in userland are different
and more variable.  Since SBT_* are undocumented, any use of them outside
the kernel is a bug and we only have to make the kernel use work.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140908131951.D880>