Date: Tue, 4 Aug 2015 15:04:53 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Jung-uk Kim <jkim@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r286265 - head/sys/x86/include Message-ID: <20150804143613.H896@besplex.bde.org> In-Reply-To: <20150804041458.GF2072@kib.kiev.ua> References: <201508040011.t740BeD3088014@repo.freebsd.org> <20150804041458.GF2072@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 4 Aug 2015, Konstantin Belousov wrote:
> On Tue, Aug 04, 2015 at 12:11:40AM +0000, Jung-uk Kim wrote:
>> Log:
>> Always define __va_list for amd64 and restore pre-r232261 behavior for i386.
>> Note it allows exotic compilers, e.g., TCC, to build with our stdio.h, etc.
Thanks.
>> PR: 201749
>> MFC after: 1 week
>>
>> Modified:
>> head/sys/x86/include/_types.h
>>
>> Modified: head/sys/x86/include/_types.h
>> ==============================================================================
>> --- head/sys/x86/include/_types.h Tue Aug 4 00:11:38 2015 (r286264)
>> +++ head/sys/x86/include/_types.h Tue Aug 4 00:11:39 2015 (r286265)
>> @@ -152,8 +152,17 @@ typedef int ___wchar_t;
>> */
>> #ifdef __GNUCLIKE_BUILTIN_VARARGS
>> typedef __builtin_va_list __va_list; /* internally known to gcc */
>> -#elif defined(lint)
>> -typedef char * __va_list; /* pretend */
>> +#else
>> +#ifdef __LP64__
>> +typedef struct {
>> + unsigned int __gpo;
>> + unsigned int __fpo;
>> + void *__oaa;
>> + void *__rsa;
>> +} __va_list;
Ugh. This is ugly and has many excical style bugs:
- tab in 'typedef struct'
- use of 'typedef struct' at all. style(9) forbids this. structs should
be declared with a tag and the used directly. That means
'#define __va_list struct __s_va_list' here
- verbose spelling of 'unsigned'
- ugly indentation from previous
- excessive underscores in struct member names. Only names in outer
scope need 2 underscores and . See stdio.h.
> What do the structure fields mean ? How is it related to the amd64 vararg
> ABI ?
It seems ABI compatible, but perhaps not API compatible. The double
underscores in the names might be to match an API. But I doubt that old
compilers even support the __LP64__ case. The certainly don't access
these names in any FreeBSD header. So the ABI could be matched by
defining __va_list as an array of 3 uint64_t's or as a struct with
unnamed fields or padding.
>> +#else
>> +typedef char * __va_list;
This still has the '*' misplaced.
>> +#endif
>> #endif
>> #if defined(__GNUC_VA_LIST_COMPATIBILITY) && !defined(__GNUC_VA_LIST) \
>> && !defined(__NO_GNUC_VA_LIST)
Is this uglyness still necessary? The gnu-style '&&' in it isn't. This
seems to be for very old versions of gcc that probably don't have builtin
varargs. In old versions of this file, the condition
defined(__GNUC_VA_LIST_COMPATIBILITY) was
defined(__GNUC__) && !defined(__INTEL_COMPILER__), so I think this ifdef
is just for defining something used in old gcc's distribution stdarg.h.
The __INTEL_COMPILER__ part of this makes no sense, and
__GNUC_VA_LIST_COMPATIBILITY is now just an obfuscated spelling of
__GNUC__. Since this is for old gcc internals, non-gcc compilers are
unlikely to need it for compatibility, so the obfuscation is just wrong.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150804143613.H896>