svn commit: r286265 - head/sys/x86/include

[Jung-uk Kim]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 08/04/2015 01:04, Bruce Evans wrote:
> 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

Fixed.  Actually, I followed sys/powerpc/include/_types.h style.  I
guess we should fix that, too.

> - verbose spelling of 'unsigned'

This file consistently uses "unsigned int" instead of "unsigned".

> - ugly indentation from previous - excessive underscores in struct
> member names.  Only names in outer scope need 2 underscores and .
> See stdio.h.

Fixed.

>> 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.

Done.  I was not really interested in API compatibility.  FYI, "Figure
3.34: va_list Type Declaration" defines the structure like this:

	typedef struct {
		unsigned int gp_offset;
		unsigned int fp_offset;
		void *overflow_arg_area;
		void *reg_save_area;
	} va_list[1];

>>> +#else +typedef    char *            __va_list;
> 
> This still has the '*' misplaced.

Note all sys/*/include/_types.h has similar style problems.

Jung-uk Kim

>>> +#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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVwQzUAAoJEHyflib82/FGbSsH/RfgNJxjeu8tJV7KE7szVLV1
bdfzGLXDs/40M4NqCjVd+9z2CFGzvfIfNXWykIwCTV0HAE6UUMb4BpirTYy9lfQF
V7y61bd6Mn6i4NbIKMksO6KZduPzGDFAbfPOX4WHOQ6IwPiG/2mzUY9oIT0Sd90D
B42Z+p1YljJfFHfaNsupL6xtYDR0SbB3avXeLaTWqf9QatTZvPatHMIncJsZ65ZD
x2FAAC7g0Xjp8DKgWiEYmbEKYQsNJgDQ1jQJlmiI3pSdp+yOdQG3RkUs+Pd54Qte
LkVdOVYRUwDIxEZIRNVcIpCEo6E+jYgRFfjZZB2v7UXVx8qoKO1qmP/RFyxRHcw=
=sj5l
-----END PGP SIGNATURE-----