Re: [3/4 revised] msi: Add partial, expandable OLE automation support.

[James Hawkins]

On 2/25/07, Misha Koshelev  wrote:
> This patch adds OLE automation support to MSI. It does this by creating
> a wrapper class, which implements any of the OLE automation classes as
> long as a function is given which is called from within the wrapper
> classes invoke method after appropriate error checking. Basic
> functionality is implemented, and it is fairly straightforward to add
> the rest as the methods are all already implemented as functions in the
> DLL.

+/*
+ * If you would like to implement a new automation function/object,
look towards the bottom of this
+ * file for the "meat and potatoes" section.
+ */
+
+/* FIXME: I don't know how big this should be */
+#define MAX_MSI_STRING 1000
+
+/*
+ * AutomationObject - "base" class for all automation objects so we
don't have to repeat functions. Just
+ *                    need to implement Invoke function for each
dispinterface that is called from within
+ *                    the AutomationObject Invoke function which
performs error checking, and pass the new
+ *                    function to create_automation_object.
+ */
+
+typedef interface AutomationObject AutomationObject;
+
+interface AutomationObject {
+    /*
+     * VTables - We provide IDispatch, IProvideClassInfo,
IProvideClassInfo2, IProvideMultipleClassInfo
+     */
+    const IDispatchVtbl *lpVtbl;
+    const IProvideClassInfoVtbl *lpvtblIProvideClassInfo;
+    const IProvideClassInfo2Vtbl *lpvtblIProvideClassInfo2;
+    const IProvideMultipleClassInfoVtbl *lpvtblIProvideMultipleClassInfo;
+
+    /* Object reference count */
+    LONG ref;
+
+    /* Clsid for this class and it's appropriate ITypeInfo object */
+    LPCLSID clsid;
+    ITypeInfo *iTypeInfo;
+
+    /* The MSI handle of the current object */
+    MSIHANDLE msiHandle;
+
+    /* A function that is called from IDispatch::Invoke, specific to
this type of object. By the
+     * time this function is called, basic error checking has been
done in the AutomationObject
+     * Invoke function */
+    HRESULT (STDMETHODCALLTYPE *funcInvoke)(
+        AutomationObject* This,
+        DISPID dispIdMember,
+        REFIID riid,
+        LCID lcid,
+        WORD wFlags,
+        DISPPARAMS* pDispParams,
+        VARIANT* pVarResult,
+        EXCEPINFO* pExcepInfo,
+        UINT* puArgErr);
+};

You went a little comment crazy here (and elsewhere).  You should tone
it down a bit.  Good code explains itself and requires little or no
commenting.

+    /*
+     * Perform a sanity check on the parameters.
+     */
+    if ( (This==0) || (ppvObject==0) )
+      return E_INVALIDARG;
+
+    /*
+     * Initialize the return parameter.
+     */
+    *ppvObject = 0;
+
+    /*
+     * Compare the riid with the interface IDs implemented by this object.
+     */
+    if (IsEqualGUID(riid, &IID_IUnknown) || IsEqualGUID(riid,
&IID_IDispatch) || IsEqualGUID(riid, This->clsid))
+        *ppvObject = This;
+    else if (IsEqualGUID(riid, &IID_IProvideClassInfo))
+	*ppvObject = (IProvideClassInfo*)&(This->lpvtblIProvideClassInfo);
+    else if (IsEqualGUID(riid, &IID_IProvideClassInfo2))
+	*ppvObject = (IProvideClassInfo2*)&(This->lpvtblIProvideClassInfo2);
+    else if (IsEqualGUID(riid, &IID_IProvideMultipleClassInfo))
+	*ppvObject = (IProvideMultipleClassInfo*)&(This->lpvtblIProvideMultipleClassInfo);
+
+    /*
+     * Check that we obtained an interface.
+     */
+    if ((*ppvObject)==0)
+    {
+	TRACE("() : asking for unsupported interface %s\n",debugstr_guid(riid));
+	return E_NOINTERFACE;
+    }
+
+    /*
+     * Query Interface always increases the reference count by one when it is
+     * successful
+     */
+    IClassFactory_AddRef(iface);
+
+    return S_OK;
+}

Way too many comments.  It may seem like nitpicking, but the comments
clutter up the code and don't provide any useful information.