svn commit: r270227 - head/sys/sys

[Bruce Evans]

On Wed, 20 Aug 2014, Xin Li wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> On 08/20/14 09:32, Davide Italiano wrote:
>> Author: davide Date: Wed Aug 20 16:32:02 2014 New Revision: 270227
>> URL: http://svnweb.freebsd.org/changeset/base/270227
>>
>> Log: Make Bruce happy removing the "LL abomination" from time.h
>> It's not necessary in all the three instances because they already
>> have the correct type on all the supported arches.
>
> I'm not yet 100% sure (still building with some of my changes) but
> this looks like the change that broke powerpc build, I saw:

That is a compiler bug, or excessive compiler flags to force the compiler
to be broken.  I thought that such compilers and/or flags went away.
However, apparently 15 years of C99 and 25 years of gnu89 is not enough
for them to go away.  We used to almost-intentionally ask for the warning
and not ask for pure gnu89 using some flag like -std=c89.  Otherwise, old
compilers default to gnu89 and support constants whos natural type is
long long.

> In file included from
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/cddl/compat/opensolaris/sys/time.h:32,
>                 from
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/sys/stat.h:99,
>                 from
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/cddl/compat/opensolaris/sys/stat.h:33,
>                 from
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../cddl/contrib/opensolaris/cmd/lockstat/lockstat.c:41:
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/sys/time.h: In
> function 'timespec2bintime':
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/sys/time.h:187:
> warning: integer constant is too large for 'long' type
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/sys/time.h: In
> function 'timeval2bintime':
> /tank/delphij/head/cddl/usr.sbin/lockstat/../../../sys/sys/time.h:204:
> warning: integer constant is too large for 'long' type
> *** [lockstat.o] Error code 1
>
>> Requested by:	bde
>>
>> Modified: head/sys/sys/time.h
>>
>> Modified: head/sys/sys/time.h
>> ==============================================================================
> - --- head/sys/sys/time.h	Wed Aug 20 16:09:05 2014	(r270226)
>> +++ head/sys/sys/time.h	Wed Aug 20 16:32:02 2014	(r270227) @@
>> -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
>> 0x7fffffffffffffffLL +#define	SBT_MAX	0x7fffffffffffffff

This is the part touched by davide recently.

>> static __inline int sbintime_getsec(sbintime_t _sbt) @@ -184,7
>> +184,7 @@ timespec2bintime(const struct timespec *
>>
>> _bt->sec = _ts->tv_sec; /* 18446744073 = int(2^64 / 1000000000) */
>> -	_bt->frac = _ts->tv_nsec * (uint64_t)18446744073LL; +	_bt->frac =
>> _ts->tv_nsec * (uint64_t)18446744073; }
>>
>> static __inline void @@ -201,7 +201,7 @@ timeval2bintime(const
>> struct timeval *_t
>>
>> _bt->sec = _tv->tv_sec; /* 18446744073709 = int(2^64 / 1000000) */
>> -	_bt->frac = _tv->tv_usec * (uint64_t)18446744073709LL; +
>> _bt->frac = _tv->tv_usec * (uint64_t)18446744073709; }
>>
>> static __inline struct timespec

Older parts used the uint64_t casts to get the correct type, but also
used the long long abomination to avoid the warning, since this used
to be necessary for gcc on i386.

It is actually -std=c99 or clang that is needed to avoid the warning.

The default for gcc is to warn, and even -std=gnu89 doesn't prevent
the warning.  This is bogus since old gcc and gnu89 support long
long and don't warn about other uses of it.

clang is more seriously broken in the other direction: it doesn't warn
about either the constants too large for long or for use of long
long even with -std=c89.  It takes -std=c89 -pedantic to get a warning
about use of long long, and this is not enough for a warning about
large constants (e.g., in 'long long x = 111111111111111;').  clang
does give a more useful conversion warning which such a constant
is assigned to a type too small to represent it.

-std=c99 is standard in kern.pre.mk but not so standard for applications.
The default for applications is -std=gnu99 but there is support for
overriding this.  So another bug bites.  I consider the functions with
the LL's in them as namespace pollution for applications.  I think
they are meant to be used by applications, but they are undocumented.
So it was not enough to check that this change doesn't break kernels.

The SBT macros are also undocumented namespace pollution for applications
(everything is under __BSD_VISIBLE, but that is the default), but since
they are macros they don't cause problems unless they are used or have
a conflicting name.  Inline functions give much more pollution than
macros since they are always parsed and they may depend on other headers.

Bruce