Re: svn commit: r270227 - head/sys/sys
Hey! On 20 August 2014 18:32, Davide Italiano dav...@freebsd.org wrote: - _bt-frac = _ts-tv_nsec * (uint64_t)18446744073LL; + _bt-frac = _ts-tv_nsec * (uint64_t)18446744073; You could also consider using UINT64_C(18446744073); that's the C standard way of creating an integer constant having a certain type. -- Ed Schouten e...@80386.nl ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
On Tue, 26 Aug 2014, Ed Schouten wrote: On 20 August 2014 18:32, Davide Italiano dav...@freebsd.org wrote: - _bt-frac = _ts-tv_nsec * (uint64_t)18446744073LL; + _bt-frac = _ts-tv_nsec * (uint64_t)18446744073; You could also consider using UINT64_C(18446744073); that's the C standard way of creating an integer constant having a certain type. That would be a further obfuscation. The *INTnC() macros expand to integer constant expressions of the specified type suitable for use in #if preprocessing directives. (It is otherwise difficult to detemine the correct suffix, to add to the constant to give it the specified type). There are no preprocessing directives here, so a simple cast works. The cast could also be applied to the other operand but it is easier to read when applied to the constant. UINT64_C() might work around the compiler bug of warning for constants larger than ULONG_MAX, depending on its implementation. I think it always does. On 64-bit arches, the above constant is not larger than ULONG_MAX so there is no problem, and on 32-bit arches the implementation can't be much different from appending 'ULL'. The expression could also be written without a cast and without using UINT64_C(), by using a 'ULL' suffix instead of 'LL'. That would still use the long long abomination, and be different obfuscation -- the type of the constant doesn't really matter, but we need to promote to the type of 'frac', that is, uint64_t. 'ULL' works because long longs are at least 64 bits (and I think unsigned long longs are also 2's complemention, so their type is larger than uint64_t. Bruce ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
On Tue, Aug 26, 2014 at 9:58 AM, Bruce Evans b...@optusnet.com.au wrote: That would be a further obfuscation. The *INTnC() macros expand to integer constant expressions of the specified type suitable for use in #if preprocessing directives. (It is otherwise difficult to detemine the correct suffix, to add to the constant to give it the specified type). There are no preprocessing directives here, so a simple cast works. The cast could also be applied to the other operand but it is easier to read when applied to the constant. I thought that in C99, all integers in preprocessor evaluation were treated as if they were [u]intmax_t (6.10.1.4 in the n1256.pdf I have here). I was only just skimming that part yesterday for unrelated reasons, though, so maybe I'm missing the bigger picture. The expression could also be written without a cast and without using UINT64_C(), by using a 'ULL' suffix instead of 'LL'. That would still use the long long abomination, and be different obfuscation -- the type of the constant doesn't really matter, but we need to promote to the type of 'frac', that is, uint64_t. 'ULL' works because long longs are at least 64 bits (and I think unsigned long longs are also 2's complemention, so their type is larger than uint64_t. Two's complement semantics are guaranteed for the fixed width types such as int64_t, but I'm not sure how that comes into play for unsigned types? -Ben ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
On Tue, 26 Aug 2014, Benjamin Kaduk wrote: On Tue, Aug 26, 2014 at 9:58 AM, Bruce Evans b...@optusnet.com.au wrote: That would be a further obfuscation. The *INTnC() macros expand to integer constant expressions of the specified type suitable for use in #if preprocessing directives. (It is otherwise difficult to detemine the correct suffix, to add to the constant to give it the specified type). There are no preprocessing directives here, so a simple cast works. The cast could also be applied to the other operand but it is easier to read when applied to the constant. I thought that in C99, all integers in preprocessor evaluation were treated as if they were [u]intmax_t (6.10.1.4 in the n1256.pdf I have here). I was only just skimming that part yesterday for unrelated reasons, though, so maybe I'm missing the bigger picture. Yes, that makes it unclear what typed constants in cpp expressions are useful for. C has always been careful to make limits like UINT_MAX have the correct type, but that seems to be worse than useless in cpp expressions. (Oops. I was thinking that UCHAR_MAX had type u_char, but the correctness actually goes the other way -- it is required to have type the promotion of u_char (signed int except on exotic machines). UINT_MAX has type unsigned int. So -UINT_MAX 0 in normal expressions. But in cpp expressions, UINT_MAX promotes to intmax_t before it is negated (since C's broken value-preserving promotion rules apply). So -UINT_MAX 0 in cpp expressions. There seem to be some compiler bugs in this. In cpp expressions, testing on clang on amd64 gives -UINT_MAX 0 and -0x 0, but -0xU 0. The first 2 results are the same since UINT_MAX is just 0x. (limits.h intentionally avoids using suffixes on constants if possible since it didn't use them old versions, I don't like them, and I would have tried to undo any changes that added them. Some buggy versions used them to break thinks like USHRT_MAX -- 0xU has the wrong type.) But 0x and 0xU have the same type if u_int == uint32_t, since the type of an unsuffixed hex constant is the type of lowest rank that can represent it. gcc-3.3 on i386 under an old version of FreeBSD gives the same results, except UINT_MAX is defined with a U suffix so the the result for it agrees with the wrong result for 0x. Perhaps this is specified somewhere, but it is bizarre for 0xU to promote not bug for bug compatibly with the value-preserving rules, while the same type and value spelled as 0x does promote bug for bug compatibly. TenDRA (4.2) is normally more of a C compiler than gcc or clang, and it gives very interesting errors for all 3 cpp expressions: [ISO C90 6.8.1]: Can't have target dependent '#if' at outer level. The expression is target-dependent since the type of 0x depends on the size of int. Even the result of -1U 0 is apparently target- dependent in C90. I think it is still target-dependent. On exotic targets, uintmax_t == u_int, so there is no promotion and -1U 0, but on normal targets 1U promotes to a (intmax_t)1 and negating that makes it negative. I didn't know about this stupid rule 6.8.1. It mainly prevents you writing target-dependent cpp expressions to determine the target. It seems to be a bug in TenDRA. 6.8.1 only says that it is implementation-defined whether the result is the same as a non-cpp expression, and gives an example of a more usefule expression involving character constants being quite likely to give a different rewsult. Sigh. My compiler and TurboC handled cpp expressions better in 1988. cpp was part of the compiler, so casts, sizeof() and floating point worked in it, and the result of constant expressions didn't depend on whether they were evaluated in cpp. Running my compiler now gives -0x 0 for all spellings of 0x, since the compiler is pre-C90 and never implemented value-preserving promotion. It also never supported 64-bit integers, so promotion doesn't apply in this example, but it applies to -0x in 16-bit mode similarly. The expression could also be written without a cast and without using UINT64_C(), by using a 'ULL' suffix instead of 'LL'. That would still use the long long abomination, and be different obfuscation -- the type of the constant doesn't really matter, but we need to promote to the type of 'frac', that is, uint64_t. 'ULL' works because long longs are at least 64 bits (and I think unsigned long longs are also 2's complemention, so their type is larger than uint64_t. Two's complement semantics are guaranteed for the fixed width types such as int64_t, but I'm not sure how that comes into play for unsigned types? Unsigned types have 2's complement arithmetic, but might not have purely 2's complement representations (except for unsigned char). They can have padding bits, Perhaps with trap representations. I think that is the only complication allowed in in
svn commit: r270227 - head/sys/sys
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. 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 _ #defineSBT_1MS (SBT_1S / 1000) #defineSBT_1US (SBT_1S / 100) #defineSBT_1NS (SBT_1S / 10) -#defineSBT_MAX 0x7fffLL +#defineSBT_MAX 0x7fff 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 / 10) */ - _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 / 100) */ - _bt-frac = _tv-tv_usec * (uint64_t)18446744073709LL; + _bt-frac = _tv-tv_usec * (uint64_t)18446744073709; } static __inline struct timespec ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
On Wed, Aug 20, 2014 at 9:32 AM, Davide Italiano dav...@freebsd.org 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. Requested by: bde CR: https://reviews.freebsd.org/D434 ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
-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: 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 / 100) #define SBT_1NS (SBT_1S / 10) -#define SBT_MAX 0x7fffLL +#define SBT_MAX 0x7fff 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 / 10) */ - _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 / 100) */ - _bt-frac = _tv-tv_usec * (uint64_t)18446744073709LL; + _bt-frac = _tv-tv_usec * (uint64_t)18446744073709; } static __inline struct timespec - -- Xin LI delp...@delphij.nethttps://www.delphij.net/ FreeBSD - The Power to Serve! Live free or die -BEGIN PGP SIGNATURE- Version: GnuPG v2.0 iQIcBAEBCgAGBQJT9PoeAAoJEJW2GBstM+nsvAsP/3asjb/pBnK4jXnouHSSHmzf Hww9RbqtRrLVGmxl0utcI4yhs/5yvlm2wqGyhfqS4EY8dHEXE0N+gtHTXcZsrCSM g/3kJLGNj8oA6poNWXiNWo4fIVEmsOgwy/K2ef5EDs8moq19dP7/3x7ixjpEMIIz 80Yq9Zts1hNLICKlv2gKCONpGo5MyThquJytadXsRKz+McZnzR3XaXnHEIl5DB9+ OgZrKuYX4aGdWguppciS53IUvdn43jF5NZw+JFsf6SwXX5I7p4DjM1JE0R6wssKr jeuelME0xmki462VBlOBj+ul/JHEbF6aF8Nkp4VVf0oN527rRguJ1Qu7a2r/xbO1 Vcc28rO8o+oxbYotnCItlKtj2tXBivalc+gcHV+ySdVL/mIDAcPw0UVqYWIQAQul QHRW+zwnnVH5DRoHn0IBpDs1jlvu45xubc0iVpZPWHbE3FFt8HMcrqMQc+7ppSAn HJSQv4kxEhvR/jHzu3dY6cnziWsz5TZT4V4HfE9ut7QZELA5f95DR5HqT82dOf0x 4Wrc9uJT2yJayvK5E40/tuXMhipv+OOm/Gjs1gF/BHULHjMEc1LRRirwY0DAewwd 8p/cTmKNXpyTxi24kAvJ2mWKNdv4ncrjWIgLDJLZfMruA4SbaD3TCZEs0evmE1lH gbdwjyzcWuwHUMh+1TfG =1yEW -END PGP SIGNATURE- ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
On Wed, Aug 20, 2014 at 12:42 PM, Xin Li delp...@delphij.net 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: No problem to (at least partly) revert this if it causes problem. Let me know. -- Davide There are no solved problems; there are only problems that are more or less solved -- Henri Poincare ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
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 / 100) #define SBT_1NS (SBT_1S / 10) -#define SBT_MAX 0x7fffLL +#define SBT_MAX 0x7fff 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 / 10) */ - _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 / 100) */ - _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 = 111;'). 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 ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r270227 - head/sys/sys
On Thu, 21 Aug 2014, Bruce Evans wrote: 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. [It is actually -std=c99 or clang that is needed to avoid the warning.] +184,7 @@ timespec2bintime(const struct timespec * _bt-sec = _ts-tv_sec; /* 18446744073 = int(2^64 / 10) */ - _bt-frac = _ts-tv_nsec * (uint64_t)18446744073LL; + _bt-frac = _ts-tv_nsec * (uint64_t)18446744073; } ... 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. I also wished for the correct fix of spelling the magic decimal numbers non-magically and without a comment as is already done for some numbers elsewhere in the file. The magic 1844mumble is 2**64 obfuscated by spelling it in decimal. I can only remember what 2**N is in decimal up to N = 16 and calculate it easily up to N = 20. 2**64 is best written as (uint65_t)1 64, but since most arches don't have uint65_t you can only write 2**63 using as (uint64_t)1 63 this method. Dividing by 2 is easy and gives small or recognizable decimal constants: /* No comment: */ _bt-frac = _ts-tv_nsec * (((uint64_t)1 63) / 5); Strictly portable code also needs an L suffix on the 5, since ints might be 16 bits and then 5 would be too large for an int constant and compilers might gratuitously warn about this. FreeBSD doesn't need this since it only supports 32-bit ints. Bruce ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
