<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Fabian,<br>
</p>
<br>
<div class="moz-cite-prefix">On 09/11/2018 18:54, Fabian Maurer
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">This is a first try to generalize COM implementations.
I find it useful to not repeat all that code for every
new implementation.
Feedback appreciated, can we go that route?</pre>
</blockquote>
<br>
Sure, some thoughts are bellow. Since you sent widl version later,
let me concentrate on class factories, as it might be useful in widl
conversation as well.<br>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">diff --git a/include/wine/com.h b/include/wine/com.h
new file mode 100644
index 0000000000..2f73bfcbd0
--- /dev/null
+++ b/include/wine/com.h</pre>
</blockquote>
<p><br>
</p>
<p> I'm skeptical about this being general COM helper. A header just
for class factory could be better.<br>
</p>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">@@ -0,0 +1,256 @@
+/*
+ * COM helper functions
+ *
+ * Copyright 2018 Fabian Maurer
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#ifndef __WINE_WINE_COM_H
+#define __WINE_WINE_COM_H
+
+#define COBJMACROS
+#include "unknwn.h"
+
+#include <stdarg.h>
+#include "windef.h"
+#include "winbase.h"
+#include "winuser.h"
+#include "winreg.h"
+#include "rpcproxy.h"
+
+#include "wine/debug.h"
+
+#ifdef __WINE_WINE_TEST_H
+#error This file should not be used in Wine tests
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+static HINSTANCE instance_dll;
+
+#ifdef WINE_COM_MAIN
+
+typedef struct
+{
+ const CLSID *clsid;
+ HRESULT (*pfnCreateInstance)(IUnknown *unk_outer, void **ppobj);
+} com_object_creation_info;</pre>
</blockquote>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">+
+typedef struct
+{
+ IClassFactory IClassFactory_iface;
+
+ LONG ref;</pre>
</blockquote>
<br>
Class factories don't really need to be heap-based. If they are
static, they don't even need ref count.<br>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">+ HRESULT (*pfnCreateInstance)(IUnknown *unk_outer, void **ppobj);</pre>
</blockquote>
<br>
This could be expressed directly in IClassFactoryVtbl by having
different vtbls for each implemented class factory.<br>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">+} IClassFactoryImpl;</pre>
</blockquote>
<br>
With both above combined, you don't really need IClassFactoryImpl
struct, which simplifies implementation a bit.<br>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">+
+static inline BOOL WINAPI com_main(HINSTANCE instance, DWORD reason, LPVOID reserved)
+{
+ TRACE("(0x%p, %d, %p)\n", instance, reason, reserved);
+
+ switch (reason)
+ {
+ case DLL_WINE_PREATTACH:
+ return FALSE; /* prefer native version */
+ case DLL_PROCESS_ATTACH:
+ DisableThreadLibraryCalls(instance);
+ instance_dll = instance;
+ break;
+ }
+
+ return TRUE;
+}</pre>
</blockquote>
<br>
I'm not convinced we need that. Even if we do, it has nothing to do
with COM. This could be done by exposing module instance from
winecrt0.<br>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">+static HRESULT WINAPI classfactory_LockServer(IClassFactory *iface, BOOL dolock)
+{
+ IClassFactoryImpl *impl = impl_from_IClassFactory(iface);
+ FIXME("(%p)->(%d), stub!\n", impl, dolock);</pre>
</blockquote>
<br>
Having FIXME in generic implementation is not nice. Most Wine DLLs
just returns S_FALSE from DllCanUnloadNow anyway and for them doing
nothing in LockServer is perfectly fine.<br>
<br>
<blockquote type="cite"
cite="mid:20181109175428.17757-1-dark.shadow4@web.de">
<pre wrap="">+ return S_OK;
+}
+
+static const IClassFactoryVtbl classfactory_Vtbl =
+{
+ classfactory_QueryInterface,
+ classfactory_AddRef,
+ classfactory_Release,
+ classfactory_CreateInstance,
+ classfactory_LockServer
+};
+
+HRESULT WINAPI com_get_class_object(const com_object_creation_info *object_creation, int object_creation_length,
+ REFCLSID rclsid, REFIID riid, void **ppv)
+{
+ unsigned int i;
+ IClassFactoryImpl *factory;
+
+ TRACE("(%s,%s,%p)\n", debugstr_guid(rclsid), debugstr_guid(riid), ppv);
+
+ if (!IsEqualGUID(&IID_IClassFactory, riid)
+ && !IsEqualGUID( &IID_IUnknown, riid))
+ return E_NOINTERFACE;
+
+ for (i = 0; i < object_creation_length; i++)
+ {
+ if (IsEqualGUID(object_creation[i].clsid, rclsid))
+ break;
+ }
+
+ if (i == object_creation_length)
+ {
+ FIXME("%s: no class found.\n", debugstr_guid(rclsid));
+ return CLASS_E_CLASSNOTAVAILABLE;
+ }
+
+ factory = HeapAlloc(GetProcessHeap(), 0, sizeof(*factory));
+ if (factory == NULL)
+ return E_OUTOFMEMORY;
+
+ factory->IClassFactory_iface.lpVtbl = &classfactory_Vtbl;
+ factory->ref = 1;
+
+ factory->pfnCreateInstance = object_creation[i].pfnCreateInstance;
+
+ *ppv = &factory->IClassFactory_iface;
+ return S_OK;
+}</pre>
</blockquote>
<br>
This is much more complicated than it needs to be. Even after
removing dynamic allocation, I'm not convinced that CLSID lookup is
worth the complication.<br>
<br>
<br>
Here is an idea about how implementing class factory could look
like:<br>
<br>
#include "wine/factory."<br>
<br>
extern WINAPI object_constructor(IClassFactory*, REFIID, void**);<br>
<br>
CLASS_FACTORY_IMPL(example_cf, object_constructor);<br>
<br>
HRESULT WINAPI DllGetClassObject(REFCLSID clsid, REFIID riid, void
**ppv)<br>
{<br>
If (IsEqualGUID(GUID_Example, clsid))<br>
return IClassFactory_QueryInterface(&example_cf, riid,
ppv);<br>
return CLASS_E_CLASSNOTAVAILABLE;<br>
}<br>
<br>
The actual code would be static functions inside the header and
CLASS_FACTORY_IMPL macro would just fill the vtbl using supplied
constructor for CreateInstance. How does that look to you?<br>
<br>
Thanks,<br>
Jacek<br>
</body>
</html>