From owner-svn-src-projects@FreeBSD.ORG Sun May 20 08:58:30 2012 Return-Path: Delivered-To: svn-src-projects@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EC901106564A; Sun, 20 May 2012 08:58:30 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx10.syd.optusnet.com.au (fallbackmx10.syd.optusnet.com.au [211.29.132.251]) by mx1.freebsd.org (Postfix) with ESMTP id 2DF458FC08; Sun, 20 May 2012 08:58:29 +0000 (UTC) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by fallbackmx10.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q4K8wNDd030784; Sun, 20 May 2012 18:58:23 +1000 Received: from c122-106-171-232.carlnfd1.nsw.optusnet.com.au (c122-106-171-232.carlnfd1.nsw.optusnet.com.au [122.106.171.232]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q4K8wCef028222 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 20 May 2012 18:58:14 +1000 Date: Sun, 20 May 2012 18:58:12 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrew Turner In-Reply-To: <201205192351.q4JNpnAq053531@svn.freebsd.org> Message-ID: <20120520165107.D822@besplex.bde.org> References: <201205192351.q4JNpnAq053531@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-projects@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r235672 - in projects/arm_eabi/sys: amd64/include i386/include ia64/include mips/include pc98/include powerpc/include sparc64/include x86/include X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 20 May 2012 08:58:31 -0000 On Sat, 19 May 2012, Andrew Turner wrote: > Log: > Fix wchar support in the not ARM case. > > * Add machine/_wchar.h to define WCHAR_{MIN,MAX} and include it from > machine/_stdint.h, it is already in wchar.h. This adds 2 layers of include pessimizations to x86, though only 1 layer to other arches. The pessimization is most noticeable over nfs, where the RPCs for reopening include files can take 10-100 times longer than actually reading and parsing of the files for small files. This has some style bugs. > * Add the typedef for __wchar_t to machine/_types.h. The limits should be defined (with leading underscores) here too, so that no new includes are needed. > Added: projects/arm_eabi/sys/amd64/include/_wchar.h > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ projects/arm_eabi/sys/amd64/include/_wchar.h Sat May 19 23:51:48 2012 (r235672) > @@ -0,0 +1,6 @@ > +/*- > + * This file is in the public domain. > + */ > +/* $FreeBSD$ */ > + > +#include Going through x86 gives the second layer. Having an extra layer is especially ugly for new files (nothing to be compatible with), but seems to be needed even for them so that upper layers can keep using the generic pathname ). > Modified: projects/arm_eabi/sys/ia64/include/_stdint.h > ============================================================================== > --- projects/arm_eabi/sys/ia64/include/_stdint.h Sat May 19 23:25:57 2012 (r235671) > +++ projects/arm_eabi/sys/ia64/include/_stdint.h Sat May 19 23:51:48 2012 (r235672) > @@ -52,6 +52,8 @@ > > #if !defined(__cplusplus) || defined(__STDC_LIMIT_MACROS) > > +#include > + This should be done at a higher, MI level (hopefully just in and ). > /* > * ISO/IEC 9899:1999 > * 7.18.2.1 Limits of exact-width integer types > @@ -149,12 +151,6 @@ > /* Limit of size_t. */ > #define SIZE_MAX UINT64_MAX > > -#ifndef WCHAR_MIN /* Also possibly defined in */ > -/* Limits of wchar_t. */ > -#define WCHAR_MIN INT32_MIN > -#define WCHAR_MAX INT32_MAX > -#endif > - Once the limits are defined (with leading underscores) in machine/_types.h, everywhere that needs to define them without leading underscores can simply repeat the definitions without the ifdefs. This is not such a place, since the application-visible definition is only needed in sys/stdint.h (where you moved it). > /* Limits of wint_t. */ > #define WINT_MIN INT32_MIN > #define WINT_MAX INT32_MAX WINT_* will probably need the same treatment eventually. For the other limits, most are still declared without going through underscores in this file (machine/_stdint.h). Declaring them (with leading underscores) next to their typedefs in machine/_types.h doesn't work so well since it replaces a layer of include nesting/ obfuscation by a layer of #define nesting/obfuscation for no reason. For WCHAR_*, we need the layer of #define nesting for technical (anti-namespace-pollution) reasons. Elsewhere I complained about missing anti-namespace pollution in sys/time.h. It would be painful to do correctly. This commit does it correctly for just 2 symbols, but takes about 400 lines. That is too painful by a factor of about 100. The existence of sys/_stdint.h is an older quality of implementation bug for . It exists just to avoid duplicating (twice) 11 ifdefs that used to be in and (some ifdefs are needed since typedefs can't be repeated, unlike #defines). There is still a maze of ifdefs, and sys/_stdint.h increases the maze for the stdint.h includes too. The existence if sys/_null.h is an even older pessimization related to this one. It is not even clean, since it has MD ifdefs at the MI level. Better yet, its original reason for existence has been subverted so that it has been reduced to just C++ support, with a different and worse unconditional definition of NULL for C. Originally it gave a mixture of (conditional) definitions for C, with the mixture being intended primarily for hiding bugs on LP64 systems, but also helping to detect bugs by compiling the same sources on different arches. Now it gives a similar mixture for C++, but only for compatibility with old compilers. > Modified: projects/arm_eabi/sys/ia64/include/_types.h > ============================================================================== > --- projects/arm_eabi/sys/ia64/include/_types.h Sat May 19 23:25:57 2012 (r235671) > +++ projects/arm_eabi/sys/ia64/include/_types.h Sat May 19 23:51:48 2012 (r235672) > @@ -115,4 +115,6 @@ typedef char * __va_list; /* non-funct > #endif /* lint */ > #endif /* __GNUCLIKE_BUILTIN_VARARGS */ > > +typedef int __wchar_t; > + Style bug: new type in a new section after the gnu section, instead of sorted into the old general section. Similarly elsewhere. > #endif /* !_MACHINE__TYPES_H_ */ > > Added: projects/arm_eabi/sys/ia64/include/_wchar.h > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ projects/arm_eabi/sys/ia64/include/_wchar.h Sat May 19 23:51:48 2012 (r235672) > @@ -0,0 +1,37 @@ > +/* Style bug: missing indent protection. The copyright template /usr/share/examples/etc/bsd-style-copyright is supposed to prevent this style bug by not having it. > + * Copyright (C) 2011 Andrew Turner The copyright template also has a lower case (C) here, but otherwise seems to be identical with this one. > ... > + * $FreeBSD$ > + */ These little include files aren't so little when they have big copyrights in them. They may take hundreds if not thousands of nanoseconds to process. Include nesting gives tens if not hundreds of them per top-level include. > + > +#ifndef _IA64_INCLUDE__WCHAR_H_ > +#define _IA64_INCLUDE__WCHAR_H_ Style bug: space instead of tab after #define. Style bug: the idempotency guard for files in the directory is conventionally named _MACHINE_FOO_H_, not _${ARCH}_INCLUDE_FOO_H. This rule seems to have been followed in most cases. However, in x86/include, copying from the directory has resulted in _MACHINE_ being used for many files that aren't in the directory. itself is intentionally generic, but especially with include convolutions like the ones for and 32-bit sub-APIs and maybe arm endianness sub-APIs, you may want non-generic names to help untangle the mess. > + > +/* Limits of wchar_t. */ > +#define WCHAR_MIN __INT_MIN > +#define WCHAR_MAX __INT_MAX There is a minor scope problem here. This file is not self-sufficient, since it depends on machine/_limits.h. Moving the definitions to machine/_types.h doesn't change this problem, but gives another reason why _all_ the limits should be defined near their typedefs there. > + > +#endif /* _IA64_INCLUDE__WCHAR_H_ */ Style bug: backwards comment on #endif. > + Style bug: extra blank line before EOF. But this file is not needed. Similarly elsewhere. Bruce