[PATCH 1/5] avicap32: Define NONAMELESSUNION in files that include objidl.h.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION. dlls/avicap32/avicap32_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/avicap32/avicap32_main.c b/dlls/avicap32/avicap32_main.c index 9e2a99d4c7c..0178dc87edb 100644 --- a/dlls/avicap32/avicap32_main.c +++ b/dlls/avicap32/avicap32_main.c @@ -43,6 +43,7 @@ # include <unistd.h> #endif +#define NONAMELESSUNION #include "windef.h" #include "winbase.h" #include "wingdi.h" -- 2.24.0
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/adsiid/adsiid.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/adsiid/adsiid.c b/dlls/adsiid/adsiid.c index fc0b8b1bf4e..69aeb6a9a32 100644 --- a/dlls/adsiid/adsiid.c +++ b/dlls/adsiid/adsiid.c @@ -20,6 +20,7 @@ #include <stdarg.h> +#define NONAMELESSUNION #include "windef.h" #include "objbase.h" #include "initguid.h" -- 2.24.0
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/security.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c index a7707f15d39..3ed27bfe8fd 100644 --- a/dlls/advapi32/security.c +++ b/dlls/advapi32/security.c @@ -25,6 +25,7 @@ #include <string.h> #include "ntstatus.h" +#define NONAMELESSUNION #define WIN32_NO_STATUS #include "windef.h" #include "winbase.h" -- 2.24.0
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/cabinet/cabinet_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/cabinet/cabinet_main.c b/dlls/cabinet/cabinet_main.c index f95eca93c52..f6017fc7eb2 100644 --- a/dlls/cabinet/cabinet_main.c +++ b/dlls/cabinet/cabinet_main.c @@ -23,6 +23,7 @@ #include <string.h> #include <fcntl.h> +#define NONAMELESSUNION #include "windef.h" #include "winbase.h" #include "winerror.h" -- 2.24.0
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/dbghelp/coff.c | 7 +------ dlls/dbghelp/dbghelp_private.h | 1 + dlls/dbghelp/stabs.c | 5 ----- dlls/dbghelp/storage.c | 2 +- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/dlls/dbghelp/coff.c b/dlls/dbghelp/coff.c index 01c5826c7ff..197434f1f31 100644 --- a/dlls/dbghelp/coff.c +++ b/dlls/dbghelp/coff.c @@ -43,14 +43,9 @@ # include <unistd.h> #endif -#include <stdarg.h> -#include "windef.h" -#include "winbase.h" -#include "winternl.h" - +#include "dbghelp_private.h" #include "wine/exception.h" #include "wine/debug.h" -#include "dbghelp_private.h" #include "wine/mscvpdb.h" WINE_DEFAULT_DEBUG_CHANNEL(dbghelp_coff); diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h index b52392aeced..902bc114bb5 100644 --- a/dlls/dbghelp/dbghelp_private.h +++ b/dlls/dbghelp/dbghelp_private.h @@ -22,6 +22,7 @@ */ #include <stdarg.h> +#define NONAMELESSUNION #include "windef.h" #include "winbase.h" #include "winver.h" diff --git a/dlls/dbghelp/stabs.c b/dlls/dbghelp/stabs.c index bc5ef356e8c..55116735f5f 100644 --- a/dlls/dbghelp/stabs.c +++ b/dlls/dbghelp/stabs.c @@ -49,16 +49,11 @@ #endif #include <stdio.h> #include <assert.h> -#include <stdarg.h> #ifdef HAVE_MACH_O_NLIST_H # include <mach-o/nlist.h> #endif -#include "windef.h" -#include "winbase.h" -#include "winnls.h" - #include "dbghelp_private.h" #include "wine/debug.h" diff --git a/dlls/dbghelp/storage.c b/dlls/dbghelp/storage.c index b08639f5518..cd5792ca413 100644 --- a/dlls/dbghelp/storage.c +++ b/dlls/dbghelp/storage.c @@ -23,9 +23,9 @@ #include "config.h" #include <assert.h> #include <stdlib.h> -#include "wine/debug.h" #include "dbghelp_private.h" +#include "wine/debug.h" #ifdef USE_STATS #include <math.h> #endif -- 2.24.0
Zebediah Figura <z.figura12(a)gmail.com> writes:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION.
I'm not sure I understand what fix you are proposing, but if it requires Winelib users to also define NONAMELESSUNION I don't think it's a viable solution. -- Alexandre Julliard julliard(a)winehq.org
On 12/17/19 11:37 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION.
I'm not sure I understand what fix you are proposing, but if it requires Winelib users to also define NONAMELESSUNION I don't think it's a viable solution.
Essentially, this: --- diff --git a/include/objidl.idl b/include/objidl.idl index a0ad1f6e01f..4c56c422747 100644 --- a/include/objidl.idl +++ b/include/objidl.idl @@ -1427,7 +1427,7 @@ interface IAdviseSink : IUnknown } GDI_OBJECT; typedef struct _userSTGMEDIUM { - union switch(DWORD tymed) u { + union _STGMEDIUM_UNION switch(DWORD tymed) u { case TYMED_NULL: ; case TYMED_MFPICT: wireHMETAFILEPICT hMetaFilePict; case TYMED_ENHMF: wireHENHMETAFILE hHEnhMetaFile; --- Which causes warnings anywhere that nameless unions are used: --- In file included from ../../../wine/include/objbase.h:258, from ../../../wine/include/ole2.h:25, from ../../include/strmif.h:13, from ../../../wine/dlls/wmcodecdspuuid/wmcodecdspuuid.c:22: ../../include/objidl.h:6109:21: warning: declaration does not declare anything 6109 | } DUMMYUNIONNAME; | ^ ---
Zebediah Figura <zfigura(a)codeweavers.com> writes:
On 12/17/19 11:37 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION.
I'm not sure I understand what fix you are proposing, but if it requires Winelib users to also define NONAMELESSUNION I don't think it's a viable solution.
Essentially, this:
--- diff --git a/include/objidl.idl b/include/objidl.idl index a0ad1f6e01f..4c56c422747 100644 --- a/include/objidl.idl +++ b/include/objidl.idl @@ -1427,7 +1427,7 @@ interface IAdviseSink : IUnknown } GDI_OBJECT;
typedef struct _userSTGMEDIUM { - union switch(DWORD tymed) u { + union _STGMEDIUM_UNION switch(DWORD tymed) u { case TYMED_NULL: ; case TYMED_MFPICT: wireHMETAFILEPICT hMetaFilePict; case TYMED_ENHMF: wireHENHMETAFILE hHEnhMetaFile; ---
Which causes warnings anywhere that nameless unions are used:
--- In file included from ../../../wine/include/objbase.h:258, from ../../../wine/include/ole2.h:25, from ../../include/strmif.h:13, from ../../../wine/dlls/wmcodecdspuuid/wmcodecdspuuid.c:22: ../../include/objidl.h:6109:21: warning: declaration does not declare anything 6109 | } DUMMYUNIONNAME;
I'm afraid we'll have to find a different fix then. -- Alexandre Julliard julliard(a)winehq.org
On 12/17/19 12:42 PM, Alexandre Julliard wrote:
Zebediah Figura <zfigura(a)codeweavers.com> writes:
On 12/17/19 11:37 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION.
I'm not sure I understand what fix you are proposing, but if it requires Winelib users to also define NONAMELESSUNION I don't think it's a viable solution.
Essentially, this:
--- diff --git a/include/objidl.idl b/include/objidl.idl index a0ad1f6e01f..4c56c422747 100644 --- a/include/objidl.idl +++ b/include/objidl.idl @@ -1427,7 +1427,7 @@ interface IAdviseSink : IUnknown } GDI_OBJECT;
typedef struct _userSTGMEDIUM { - union switch(DWORD tymed) u { + union _STGMEDIUM_UNION switch(DWORD tymed) u { case TYMED_NULL: ; case TYMED_MFPICT: wireHMETAFILEPICT hMetaFilePict; case TYMED_ENHMF: wireHENHMETAFILE hHEnhMetaFile; ---
Which causes warnings anywhere that nameless unions are used:
--- In file included from ../../../wine/include/objbase.h:258, from ../../../wine/include/ole2.h:25, from ../../include/strmif.h:13, from ../../../wine/dlls/wmcodecdspuuid/wmcodecdspuuid.c:22: ../../include/objidl.h:6109:21: warning: declaration does not declare anything 6109 | } DUMMYUNIONNAME;
I'm afraid we'll have to find a different fix then.
I understand the difficulty, but I'm not sure what else we can reasonably do here. As far as I can see either we have to throw C++ under the bus (i.e. close that bug as WONTFIX), or we have to add specific differences into widl (or objidl.idl) for winelib, to avoid violating the C spec. That either means making the structure type anonymous (i.e. not defining _STGMEDIUM_UNION), which is not particularly great (not that it's particularly likely anyone would want to use the _STGMEDIUM_UNION type), or forcibly giving the union member a name (which would also potentially require changes in winelib user code). But maybe I'm missing another, more perfect option.
Zebediah Figura <z.figura12(a)gmail.com> writes:
On 12/17/19 12:42 PM, Alexandre Julliard wrote:
Zebediah Figura <zfigura(a)codeweavers.com> writes:
On 12/17/19 11:37 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION.
I'm not sure I understand what fix you are proposing, but if it requires Winelib users to also define NONAMELESSUNION I don't think it's a viable solution.
Essentially, this:
--- diff --git a/include/objidl.idl b/include/objidl.idl index a0ad1f6e01f..4c56c422747 100644 --- a/include/objidl.idl +++ b/include/objidl.idl @@ -1427,7 +1427,7 @@ interface IAdviseSink : IUnknown } GDI_OBJECT;
typedef struct _userSTGMEDIUM { - union switch(DWORD tymed) u { + union _STGMEDIUM_UNION switch(DWORD tymed) u { case TYMED_NULL: ; case TYMED_MFPICT: wireHMETAFILEPICT hMetaFilePict; case TYMED_ENHMF: wireHENHMETAFILE hHEnhMetaFile; ---
Which causes warnings anywhere that nameless unions are used:
--- In file included from ../../../wine/include/objbase.h:258, from ../../../wine/include/ole2.h:25, from ../../include/strmif.h:13, from ../../../wine/dlls/wmcodecdspuuid/wmcodecdspuuid.c:22: ../../include/objidl.h:6109:21: warning: declaration does not declare anything 6109 | } DUMMYUNIONNAME;
I'm afraid we'll have to find a different fix then.
I understand the difficulty, but I'm not sure what else we can reasonably do here. As far as I can see either we have to throw C++ under the bus (i.e. close that bug as WONTFIX), or we have to add specific differences into widl (or objidl.idl) for winelib, to avoid violating the C spec. That either means making the structure type anonymous (i.e. not defining _STGMEDIUM_UNION), which is not particularly great (not that it's particularly likely anyone would want to use the _STGMEDIUM_UNION type), or forcibly giving the union member a name (which would also potentially require changes in winelib user code).
But maybe I'm missing another, more perfect option.
If the issue is C++ code, this can be fixed by providing a different definition with judicious use of cpp_quote and #ifdef __cplusplus. If the issue is C code accessing the structure members, it would be OK to require that code to be modified. But we can't require modifying every file that happens to include the header without any interest in that specific structure. -- Alexandre Julliard julliard(a)winehq.org
On 12/17/19 2:47 PM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
On 12/17/19 12:42 PM, Alexandre Julliard wrote:
Zebediah Figura <zfigura(a)codeweavers.com> writes:
On 12/17/19 11:37 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47710 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- This is not currently necessary, but will be with the prescribed solution for bug 47710, viz. defining the type _STGMEDIUM_UNION.
I'm not sure I understand what fix you are proposing, but if it requires Winelib users to also define NONAMELESSUNION I don't think it's a viable solution.
Essentially, this:
--- diff --git a/include/objidl.idl b/include/objidl.idl index a0ad1f6e01f..4c56c422747 100644 --- a/include/objidl.idl +++ b/include/objidl.idl @@ -1427,7 +1427,7 @@ interface IAdviseSink : IUnknown } GDI_OBJECT;
typedef struct _userSTGMEDIUM { - union switch(DWORD tymed) u { + union _STGMEDIUM_UNION switch(DWORD tymed) u { case TYMED_NULL: ; case TYMED_MFPICT: wireHMETAFILEPICT hMetaFilePict; case TYMED_ENHMF: wireHENHMETAFILE hHEnhMetaFile; ---
Which causes warnings anywhere that nameless unions are used:
--- In file included from ../../../wine/include/objbase.h:258, from ../../../wine/include/ole2.h:25, from ../../include/strmif.h:13, from ../../../wine/dlls/wmcodecdspuuid/wmcodecdspuuid.c:22: ../../include/objidl.h:6109:21: warning: declaration does not declare anything 6109 | } DUMMYUNIONNAME;
I'm afraid we'll have to find a different fix then.
I understand the difficulty, but I'm not sure what else we can reasonably do here. As far as I can see either we have to throw C++ under the bus (i.e. close that bug as WONTFIX), or we have to add specific differences into widl (or objidl.idl) for winelib, to avoid violating the C spec. That either means making the structure type anonymous (i.e. not defining _STGMEDIUM_UNION), which is not particularly great (not that it's particularly likely anyone would want to use the _STGMEDIUM_UNION type), or forcibly giving the union member a name (which would also potentially require changes in winelib user code).
But maybe I'm missing another, more perfect option.
If the issue is C++ code, this can be fixed by providing a different definition with judicious use of cpp_quote and #ifdef __cplusplus.
If the issue is C code accessing the structure members, it would be OK to require that code to be modified. But we can't require modifying every file that happens to include the header without any interest in that specific structure.
Thanks, that helps. I guess ultimately the issue is that Microsoft made a header that violates the C spec (big surprise, huh?) unless NONAMELESSUNION is defined. Microsoft's version of the header is fine for C++. Ours is fine for C but not C++; of course, it's also missing a type definition. (A type definition which probably only exists because of the aforementioned restriction on C++ code). Probably the nicest thing to do is define _STGMEDIUM_UNION only when C++ is used. That shouldn't require any changes to existing users of the header. I'll send a patch.
participants (3)
-
Alexandre Julliard -
Zebediah Figura -
Zebediah Figura