Date: Tue, 4 Aug 2015 15:04:58 -0400 From: Jung-uk Kim <jkim@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au>, Konstantin Belousov <kostikbel@gmail.com> Cc: 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: <55C10CDA.4070006@FreeBSD.org> In-Reply-To: <20150804143613.H896@besplex.bde.org> References: <201508040011.t740BeD3088014@repo.freebsd.org> <20150804041458.GF2072@kib.kiev.ua> <20150804143613.H896@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
-----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-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55C10CDA.4070006>