Hi all,
COM clean up is going well now (thanks Michael!) and after recent
conversation about wincodecs clean up, I had an idea about how to get
rid of even more casts. We could use widl to teach C a bit more about
interface inheritance, so that child->parent interface casts would work
in similar way as current obj->iface cast. I experimented a bit and
wrote a prove of concept patch for widl (see attached widl.diff). With
this patch, COM interfaces become unions in form like this:
#ifndef WIDL_TYPE_SAFE_C_INTERFACE
interface IWICBitmapFrameDecode {
CONST_VTBL IWICBitmapFrameDecodeVtbl* lpVtbl;
};
#else
union IWICBitmapFrameDecode {
CONST_VTBL IWICBitmapFrameDecodeVtbl* lpVtbl;
IWICBitmapSource base;
};
#endif
This is an opt-in macro based solution (so compatibility with standard C
interface is not a problem). You may see how this may be used in the
attached example of patch (wincodecs.diff).
Sadly, it has its drawbacks. Having interface as union instead of struct
breaks pretty sane assumption that interface==struct. Also there is no
portable way to statically initialize the interface for object-less
static interface implementation like:
IMyInterface iface_instance = { &MyInterfaceVtbl };
Since using this union based may be controlled per-file, it may not be
such a big deal, but I personally really like such constructs, esp. in
tests.
If we decide to use this solution, there are further decisions to to
(it's just a prove of concept now!):
- Naming convention. Is 'base' good? Should we use IIfaceName_iface
instead (seems too long)
- Sometimes &iface->base.base.base constructs will happen. Should we
have all parents, instead of only direct parent, in the union, so that
we can do iface->IUnknown_iface instead?
I don't have strong opinion myself yet, so I'd like to hear others'
opinions.
Cheers,
Jacek
commit d541cb606421272065c0a2ef95343b79336d3b17
Author: Jacek Caban <ja...@codeweavers.com>
Date: Wed Jul 25 11:18:45 2012 +0200
widl: Added support for type safe C interfaces
diff --git a/include/objbase.h b/include/objbase.h
index c366a28..d54377a 100644
--- a/include/objbase.h
+++ b/include/objbase.h
@@ -204,7 +204,7 @@
#define THIS_ INTERFACE *This,
#define THIS INTERFACE *This
-#define interface struct
+#define interface __widl_interface
#ifdef __WINESRC__
#define CONST_VTABLE
diff --git a/include/rpcndr.h b/include/rpcndr.h
index 514f1ce..195c634 100644
--- a/include/rpcndr.h
+++ b/include/rpcndr.h
@@ -137,8 +137,14 @@ typedef void (__RPC_USER *NDR_RUNDOWN)(void *context);
typedef void (__RPC_USER *NDR_NOTIFY_ROUTINE)(void);
typedef void (__RPC_USER *NDR_NOTIFY2_ROUTINE)(boolean flag);
+#if !defined(__cplusplus) && defined(WIDL_TYPE_SAFE_C_INTERFACE)
+#define __widl_interface union
+#else
+#define __widl_interface struct
+#endif
+
#define DECLSPEC_UUID(x)
-#define MIDL_INTERFACE(x) struct
+#define MIDL_INTERFACE(x) __widl_interface
struct _MIDL_STUB_MESSAGE;
struct _MIDL_STUB_DESC;
@@ -226,7 +232,7 @@ typedef struct _MIDL_STUB_MESSAGE
void *pvDestContext;
NDR_SCONTEXT *SavedContextHandles;
LONG ParamNumber;
- struct IRpcChannelBuffer *pRpcChannelBuffer;
+ __widl_interface IRpcChannelBuffer *pRpcChannelBuffer;
PARRAY_INFO pArrayInfo;
ULONG *SizePtrCountArray;
ULONG *SizePtrOffsetArray;
@@ -471,7 +477,7 @@ typedef struct _FULL_PTR_XLAT_TABLES {
XLAT_SIDE XlatSide;
} FULL_PTR_XLAT_TABLES, *PFULL_PTR_XLAT_TABLES;
-struct IRpcStubBuffer;
+__widl_interface IRpcStubBuffer;
typedef ULONG error_status_t;
typedef void * NDR_CCONTEXT;
@@ -487,7 +493,7 @@ typedef struct _NDR_USER_MARSHAL_INFO_LEVEL1
ULONG BufferSize;
void * (__WINE_ALLOC_SIZE(1) __RPC_API *pfnAllocate)(SIZE_T);
void (__RPC_API *pfnFree)(void *);
- struct IRpcChannelBuffer *pRpcChannelBuffer;
+ __widl_interface IRpcChannelBuffer *pRpcChannelBuffer;
ULONG_PTR Reserved[5];
} NDR_USER_MARSHAL_INFO_LEVEL1;
@@ -668,13 +674,13 @@ RPCRTAPI void RPC_ENTRY
NdrAsyncServerCall( PRPC_MESSAGE pRpcMsg );
RPCRTAPI LONG RPC_ENTRY
- NdrStubCall2( struct IRpcStubBuffer* pThis, struct IRpcChannelBuffer*
pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
+ NdrStubCall2( __widl_interface IRpcStubBuffer* pThis, __widl_interface
IRpcChannelBuffer* pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
RPCRTAPI LONG RPC_ENTRY
- NdrStubCall( struct IRpcStubBuffer* pThis, struct IRpcChannelBuffer*
pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
+ NdrStubCall( __widl_interface IRpcStubBuffer* pThis, __widl_interface
IRpcChannelBuffer* pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
RPCRTAPI LONG RPC_ENTRY
- NdrAsyncStubCall( struct IRpcStubBuffer* pThis, struct IRpcChannelBuffer*
pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
+ NdrAsyncStubCall( __widl_interface IRpcStubBuffer* pThis, __widl_interface
IRpcChannelBuffer* pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
RPCRTAPI LONG RPC_ENTRY
- NdrDcomAsyncStubCall( struct IRpcStubBuffer* pThis, struct
IRpcChannelBuffer* pChannel, PRPC_MESSAGE pRpcMsg, DWORD * pdwStubPhase );
+ NdrDcomAsyncStubCall( __widl_interface IRpcStubBuffer* pThis,
__widl_interface IRpcChannelBuffer* pChannel, PRPC_MESSAGE pRpcMsg, DWORD *
pdwStubPhase );
RPCRTAPI void* RPC_ENTRY
NdrAllocate( PMIDL_STUB_MESSAGE pStubMsg, SIZE_T Len ) __WINE_ALLOC_SIZE(2);
@@ -706,9 +712,9 @@ RPCRTAPI unsigned char* RPC_ENTRY
RPCRTAPI void RPC_ENTRY
NdrServerInitializeMarshall( PRPC_MESSAGE pRpcMsg, PMIDL_STUB_MESSAGE
pStubMsg );
RPCRTAPI void RPC_ENTRY
- NdrServerMarshall( struct IRpcStubBuffer *pThis, struct IRpcChannelBuffer
*pChannel, PMIDL_STUB_MESSAGE pStubMsg, PFORMAT_STRING pFormat );
+ NdrServerMarshall( __widl_interface IRpcStubBuffer *pThis, __widl_interface
IRpcChannelBuffer *pChannel, PMIDL_STUB_MESSAGE pStubMsg, PFORMAT_STRING
pFormat );
RPCRTAPI void RPC_ENTRY
- NdrServerUnmarshall( struct IRpcChannelBuffer *pChannel, PRPC_MESSAGE
pRpcMsg,
+ NdrServerUnmarshall( __widl_interface IRpcChannelBuffer *pChannel,
PRPC_MESSAGE pRpcMsg,
PMIDL_STUB_MESSAGE pStubMsg, PMIDL_STUB_DESC pStubDesc,
PFORMAT_STRING pFormat, void *pParamList );
RPCRTAPI unsigned char* RPC_ENTRY
diff --git a/tools/widl/header.c b/tools/widl/header.c
index 0b2b9d5..5817fac 100644
--- a/tools/widl/header.c
+++ b/tools/widl/header.c
@@ -1094,6 +1094,7 @@ static void write_com_interface_end(FILE *header, type_t
*iface)
{
int dispinterface = is_attr(iface->attrs, ATTR_DISPINTERFACE);
const UUID *uuid = get_attrp(iface->attrs, ATTR_UUID);
+ type_t *parent_iface = type_iface_get_inherit(iface);
if (uuid)
write_guid(header, dispinterface ? "DIID" : "IID", iface->name, uuid);
@@ -1104,10 +1105,9 @@ static void write_com_interface_end(FILE *header, type_t
*iface)
fprintf(header, "MIDL_INTERFACE(\"%s\")\n", uuid_string(uuid));
else
fprintf(header, "interface ");
- if (type_iface_get_inherit(iface))
+ if (parent_iface)
{
- fprintf(header, "%s : public %s\n", iface->name,
- type_iface_get_inherit(iface)->name);
+ fprintf(header, "%s : public %s\n", iface->name, parent_iface->name);
fprintf(header, "{\n");
}
else
@@ -1125,7 +1125,7 @@ static void write_com_interface_end(FILE *header, type_t
*iface)
write_cpp_method_def(header, iface);
indentation--;
}
- if (!type_iface_get_inherit(iface))
+ if (!parent_iface)
fprintf(header, " END_INTERFACE\n");
fprintf(header, "};\n");
if (uuid)
@@ -1143,14 +1143,22 @@ static void write_com_interface_end(FILE *header,
type_t *iface)
indentation--;
fprintf(header, " END_INTERFACE\n");
fprintf(header, "} %sVtbl;\n", iface->name);
+ fprintf(header, "#ifndef WIDL_TYPE_SAFE_C_INTERFACE\n");
fprintf(header, "interface %s {\n", iface->name);
fprintf(header, " CONST_VTBL %sVtbl* lpVtbl;\n", iface->name);
fprintf(header, "};\n");
+ fprintf(header, "#else\n");
+ fprintf(header, "union %s {\n", iface->name);
+ fprintf(header, " CONST_VTBL %sVtbl* lpVtbl;\n", iface->name);
+ if(parent_iface)
+ fprintf(header, " %s base;\n", parent_iface->name);
+ fprintf(header, "};\n");
+ fprintf(header, "#endif\n");
fprintf(header, "\n");
fprintf(header, "#ifdef COBJMACROS\n");
/* dispinterfaces don't have real functions, so don't write macros for them,
* only for the interface this interface inherits from, i.e. IDispatch */
- write_method_macro(header, dispinterface ? type_iface_get_inherit(iface) :
iface, iface->name);
+ write_method_macro(header, dispinterface ? parent_iface : iface,
iface->name);
fprintf(header, "#endif\n");
fprintf(header, "\n");
fprintf(header, "#endif\n");
commit 81ff07bc60951f1c6a5e9e8af4e27b07cb78a05a
Author: Jacek Caban <ja...@codeweavers.com>
Date: Wed Jul 25 11:19:10 2012 +0200
wincodecs: Use type safe C interfaces in icoformat.c
diff --git a/dlls/windowscodecs/icoformat.c b/dlls/windowscodecs/icoformat.c
index 48a5c95..20b7241 100644
--- a/dlls/windowscodecs/icoformat.c
+++ b/dlls/windowscodecs/icoformat.c
@@ -21,6 +21,7 @@
#include <stdarg.h>
#define COBJMACROS
+#define WIDL_TYPE_SAFE_C_INTERFACE
#include "windef.h"
#include "winbase.h"
@@ -260,14 +261,14 @@ static HRESULT ReadIcoDib(IStream *stream, IcoFrameDecode
*result)
if (IsEqualGUID(&pixelformat, &GUID_WICPixelFormat32bppBGR) ||
IsEqualGUID(&pixelformat, &GUID_WICPixelFormat32bppBGRA))
{
- source = (IWICBitmapSource*)framedecode;
+ source = &framedecode->base;
IWICBitmapSource_AddRef(source);
has_alpha = TRUE;
}
else
{
hr = WICConvertBitmapSource(&GUID_WICPixelFormat32bppBGRA,
- (IWICBitmapSource*)framedecode, &source);
+ &framedecode->base, &source);
has_alpha = FALSE;
}
@@ -406,7 +407,7 @@ static HRESULT ReadIcoPng(IStream *stream, IcoFrameDecode
*result)
hr = IWICBitmapDecoder_GetFrame(decoder, 0, &sourceFrame);
if (FAILED(hr))
goto end;
- hr = WICConvertBitmapSource(&GUID_WICPixelFormat32bppBGRA,
(IWICBitmapSource*)sourceFrame, &sourceBitmap);
+ hr = WICConvertBitmapSource(&GUID_WICPixelFormat32bppBGRA,
&sourceFrame->base, &sourceBitmap);
if (FAILED(hr))
goto end;
hr = IWICBitmapFrameDecode_GetSize(sourceFrame, &result->width,
&result->height);
@@ -680,10 +681,10 @@ static HRESULT WINAPI
IcoDecoder_GetFrame(IWICBitmapDecoder *iface,
case sizeof(BITMAPINFOHEADER):
case sizeof(BITMAPV4HEADER):
case sizeof(BITMAPV5HEADER):
- hr = ReadIcoDib((IStream*)substream, result);
+ hr = ReadIcoDib(&substream->base, result);
break;
case 0x474e5089:
- hr = ReadIcoPng((IStream*)substream, result);
+ hr = ReadIcoPng(&substream->base, result);
break;
default:
FIXME("Unrecognized ICO frame magic: %x\n", magic);
