[PATCH 5/5] include: Fix conflicting definitions of NULL, wchar_t, wint_t, wctype_t.
When <basetyps.h> defines wchar_t, it must obey -f(no-)short-wchar by referring to __WCHAR_TYPE__ which may not be `unsigned short`. Similarly, windef.h defining NULL may need to use GCC's __null. Otherwise one-definition-rule problems arise depending on whether wine or libc headers are included first. Implementing these by deferring to the C89-specified libc headers, (or msvcrt's versions with -mno-cygwin) ensures matching definitions. Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com> --- include/basetyps.h | 8 +++++--- include/tchar.h | 9 +-------- include/windef.h | 11 +++++++---- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/include/basetyps.h b/include/basetyps.h index b0dd5d77f4..e6243d5a59 100644 --- a/include/basetyps.h +++ b/include/basetyps.h @@ -91,9 +91,11 @@ typedef unsigned long error_status_t; #endif #ifndef _WCHAR_T_DEFINED -#ifndef __cplusplus -typedef unsigned short wchar_t; -#endif +# ifndef __cplusplus +# define __need_wchar_t // tells GCC's stddef.h to only define wchar_t +# include <stddef.h> +# undef __need_wchar_t +# endif #define _WCHAR_T_DEFINED #endif diff --git a/include/tchar.h b/include/tchar.h index 6c9035c219..0049ea7e7a 100644 --- a/include/tchar.h +++ b/include/tchar.h @@ -232,14 +232,7 @@ extern "C" { #endif /* tchar mappings */ #ifdef _UNICODE -#if !defined(_WINT_T_DEFINED) && !defined(__WINT_TYPE__) -typedef unsigned short wint_t; -#endif - -#ifndef _WCTYPE_T_DEFINED -typedef unsigned short wctype_t; -#define _WCTYPE_T_DEFINED -#endif +#include <wchar.h> #ifndef __TCHAR_DEFINED #if defined(WINE_UNICODE_NATIVE) diff --git a/include/windef.h b/include/windef.h index 521c3ab451..e26ffe6ff4 100644 --- a/include/windef.h +++ b/include/windef.h @@ -206,11 +206,14 @@ extern "C" { /* Misc. constants. */ -#undef NULL -#ifdef __cplusplus -#define NULL 0 -#else +#ifndef NULL +#ifdef RC_INVOKED #define NULL ((void*)0) +#else +#define __need_NULL // tells GCC's stddef.h to only define NULL +#include <stddef.h> +#undef __need_NULL +#endif #endif #ifdef FALSE
When <basetyps.h> defines wchar_t, it must obey -f(no-)short-wchar by referring to __WCHAR_TYPE__ which may not be `unsigned short`. Similarly, windef.h defining NULL may need to use GCC's __null. Otherwise one-definition-rule problems arise depending on whether wine or libc headers are included first.
Implementing these by deferring to the C89-specified libc headers, (or msvcrt's versions with -mno-cygwin) ensures matching definitions.
Per the apparent concern about using <uchar.h> in windows headers... If it helps, stddef.h is a gcc header, not a glibc one i.e., in C89 (and up) it's part of the "freestanding implementation". So even with a different libc (or no libc), this header should still be provided by the underlying compiler. The use of __need_* is GCC-specific, but if those don't work we'll still get the needed types, just maybe with some extras too. C89 Section 1.7: A conforming freestanding implementation shall accept any strictly conforming program in which the use of the features specified in the library section ($4) is confined to the contents of the standard headers <float.h> , <limits.h> , <stdarg.h> ,and <stddef.h> . And jacek pointed out in IRC that tchar.h already includes <wchar.h>. Not sure how I overlooked that, but I'll clean that up and send v2 patches once we agree on how to address with the char16_t part.
Hi Kevin, On 17.07.2020 03:12, Puetz Kevin A wrote:
When <basetyps.h> defines wchar_t, it must obey -f(no-)short-wchar by referring to __WCHAR_TYPE__ which may not be `unsigned short`. Similarly, windef.h defining NULL may need to use GCC's __null. Otherwise one-definition-rule problems arise depending on whether wine or libc headers are included first.
Implementing these by deferring to the C89-specified libc headers, (or msvcrt's versions with -mno-cygwin) ensures matching definitions.
This seems to be indeed the most compatible way. It looks like __need_* macros widely supported (not only by GCC itself, but also versions provided by mingw and LLVM). We may want to support that in our msvcrt headers, if we start using it. However, it's not supported by MSVC. The fallback should still work, as you explained, but I wonder if it would be safer to guard stddef.h usage by defined(__GNUC__) and leave existing definitions for other configurations.
Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com> --- include/basetyps.h | 8 +++++--- include/tchar.h | 9 +-------- include/windef.h | 11 +++++++---- 3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/include/basetyps.h b/include/basetyps.h index b0dd5d77f4..e6243d5a59 100644 --- a/include/basetyps.h +++ b/include/basetyps.h @@ -91,9 +91,11 @@ typedef unsigned long error_status_t; #endif
#ifndef _WCHAR_T_DEFINED -#ifndef __cplusplus -typedef unsigned short wchar_t; -#endif +# ifndef __cplusplus +# define __need_wchar_t // tells GCC's stddef.h to only define wchar_t
Please avoid C++ style comments. Wine uses /* */ comments. Thanks, Jacek
-----Original Message----- From: Jacek Caban <jacek(a)codeweavers.com> Sent: Wednesday, July 22, 2020 7:35 AM To: Puetz Kevin A <PuetzKevinA(a)JohnDeere.com>; wine-devel <wine- devel(a)winehq.org> Subject: Re: [PATCH 5/5] include: Fix conflicting definitions of NULL, wchar_t, wint_t, wctype_t.
Hi Kevin,
On 17.07.2020 03:12, Puetz Kevin A wrote:
When <basetyps.h> defines wchar_t, it must obey -f(no-)short-wchar by referring to __WCHAR_TYPE__ which may not be `unsigned short`. Similarly, windef.h defining NULL may need to use GCC's __null. Otherwise one-definition-rule problems arise depending on whether wine or libc headers are included first.
Implementing these by deferring to the C89-specified libc headers, (or msvcrt's versions with -mno-cygwin) ensures matching definitions.
This seems to be indeed the most compatible way. It looks like __need_* macros widely supported (not only by GCC itself, but also versions provided by mingw and LLVM). We may want to support that in our msvcrt headers, if we start using it.
That would be a little tricky because wine's MSVCRT delegates most of stddef.h into corecrt.h. I guess one could extend the __need_* guards into corecrt.h too, but it'll get messy.
However, it's not supported by MSVC. The fallback should still work, as you explained, but I wonder if it would be safer to guard stddef.h usage by defined(__GNUC__) and leave existing definitions for other configurations.
I think if anything, I'd rather do the opposite - check _MSC_VER and give it what a definition we hope matches the Windows SDK, deferring to <stddef.h> for portability on other compilers. But I don't think this actually makes a lot of sense either. Windows SDK 10.0.17763.0 drags in windef.h -> minwindef.h -> winnt.h -> ctype.h -> corecrt.h, and also winnt.h -> guiddef.h -> string.h. By the end you're only avoiding __threadid(), __threadhandle(), offsetof(), and (for c++) std::nullptr_t, the rest of corecrt.h gets pulled in eventually anyway. And it looks like wine's winnt.h actually even includes stddef.h too (except for RC_INVOKED, which I also special-cased). So the state *after* windef.h is complete already has everything, it's just (currently) has a weird NULL for a while in the middle, and a potentially conflicting wchar_t.
Please avoid C++ style comments. Wine uses /* */ comments.
Oops, fixed.
On 22.07.2020 15:35, Puetz Kevin A wrote:
But I don't think this actually makes a lot of sense either. Windows SDK 10.0.17763.0 drags in windef.h -> minwindef.h -> winnt.h -> ctype.h -> corecrt.h, and also winnt.h -> guiddef.h -> string.h. By the end you're only avoiding __threadid(), __threadhandle(), offsetof(), and (for c++) std::nullptr_t, the rest of corecrt.h gets pulled in eventually anyway.
Oh, good catch, I didn't notice that. Then maybe we could could move existing NULL define after #include <winnt.h> and guard with #ifndef NULL (instead of #undefing)? Thanks, Jacek
But I don't think this actually makes a lot of sense either. Windows SDK 10.0.17763.0 drags in windef.h -> minwindef.h -> winnt.h -> ctype.h -> corecrt.h, and also winnt.h -> guiddef.h -> string.h. By the end you're only avoiding __threadid(), __threadhandle(), offsetof(), and (for c++) std::nullptr_t, the rest of corecrt.h gets pulled in eventually anyway.
Oh, good catch, I didn't notice that. Then maybe we could could move existing NULL define after #include <winnt.h> and guard with #ifndef NULL (instead of #undefing)?
`winecpp -fdirectives-only` is a nice way to look at how all the inclusion plays out That would leave NULL undefined for the first portion of <winnt.h> (and thus during <basetsd.h, guiddef.h, winapifamily.h). It doesn't look like that would actually break anything currently, (I see no inlines or anything in those headers that use NULL) but differs from the Windows SDK and feels weird. My thought was that, since the extra symbols will eventually get defined anyway, that even less risk if the libc doesn’t do __need_* and we get them early, so we can just use <stddef.h> right away (thus clearly and portably follow ODR). Even if __need_* doesn’t work as intended, we just get things early that we were eventually going to get anyway.
On 22.07.2020 16:19, Puetz Kevin A wrote:
But I don't think this actually makes a lot of sense either. Windows SDK 10.0.17763.0 drags in windef.h -> minwindef.h -> winnt.h -> ctype.h -> corecrt.h, and also winnt.h -> guiddef.h -> string.h. By the end you're only avoiding __threadid(), __threadhandle(), offsetof(), and (for c++) std::nullptr_t, the rest of corecrt.h gets pulled in eventually anyway.
Oh, good catch, I didn't notice that. Then maybe we could could move existing NULL define after #include <winnt.h> and guard with #ifndef NULL (instead of #undefing)? `winecpp -fdirectives-only` is a nice way to look at how all the inclusion plays out
That would leave NULL undefined for the first portion of <winnt.h> (and thus during <basetsd.h, guiddef.h, winapifamily.h). It doesn't look like that would actually break anything currently, (I see no inlines or anything in those headers that use NULL) but differs from the Windows SDK and feels weird.
My thought was that, since the extra symbols will eventually get defined anyway, that even less risk if the libc doesn’t do __need_* and we get them early, so we can just use <stddef.h> right away (thus clearly and portably follow ODR). Even if __need_* doesn’t work as intended, we just get things early that we were eventually going to get anyway.
Sure, sounds reasonable, but if we're going to going to include entire stddef.h anyway, there is no need for using __need_* macros. It's not a big deal and we will still makes sense to use it in basetyps.h, but it's always nice to simplify things (eg. someone reading the header will not have to research what __need_NULL macro does). We can include stddef.h earlier in windef.h if you think that reordering may be problematic. Thanks, Jacek
participants (2)
-
Jacek Caban -
Puetz Kevin A