Date: Thu, 21 Aug 2014 07:07:55 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: d@delphij.net Cc: Davide Italiano <davide@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r270227 - head/sys/sys Message-ID: <20140821063422.P11841@besplex.bde.org> In-Reply-To: <53F4FA1E.2000103@delphij.net> References: <201408201632.s7KGW2vF093636@svn.freebsd.org> <53F4FA1E.2000103@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140821063422.P11841>