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>