Date: Sun, 20 May 2012 18:58:12 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andrew Turner <andrew@FreeBSD.org> 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 Message-ID: <20120520165107.D822@besplex.bde.org> In-Reply-To: <201205192351.q4JNpnAq053531@svn.freebsd.org> References: <201205192351.q4JNpnAq053531@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <x86/_wchar.h>
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 <machine/>).
> 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 <machine/_wchar.h>
> +
This should be done at a higher, MI level (hopefully just in <stdint.h> and
<wchar.h>).
> /*
> * 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 <wchar.h> */
> -/* 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 <stdint.h>. It exists just to avoid duplicating (twice) 11
ifdefs that used to be in <sys/types.h> and <sys/stdint.h> (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 <machine> 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 <machine> directory has resulted in _MACHINE_ being used
for many files that aren't in the <machine> directory. <machine> itself
is intentionally generic, but especially with include convolutions like
the ones for <x86> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120520165107.D822>