On Sun, 2007-02-25 at 13:00 +0000, Robert Shearman wrote:
Misha Koshelev wrote:
+/* Macros to get pointer to AutomationObject) from the other VTables. */
Typo with extra ")" here.
+HRESULT WINAPI SessionImpl_Invoke(
AutomationObject* This,DISPID dispIdMember,REFIID riid,LCID lcid,WORD wFlags,DISPPARAMS* pDispParams,VARIANT* pVarResult,EXCEPINFO* pExcepInfo,UINT* puArgErr)+{
- WCHAR szString[MAX_MSI_STRING];
- DWORD dwLen = MAX_MSI_STRING;
- IDispatch *iDispatch = NULL;
- MSIHANDLE msiHandle;
- LANGID langId;
- UINT ret;
- INSTALLSTATE iInstalled, iAction;
Some error checking here would be good, like you do in AutomationObject_Invoke.
Well perhaps there is a little confusion here as these functions are actually always called from _within_ AutomationObject_Invoke, hence the error checking is already done. I did this as there is quite a few objects to implement and to save writing the same code over and over I thought this would be a good idea. Do you think I should rename these functions to something else so it is more clear that they are not actually called directly through an IDispatch interface but rather from an AutomationObject?
- switch (dispIdMember)
- {
...
+/*
- AutomationObject - "base" class for all automation objects so we don't have to repeat functions. Just
need to implement Invoke function for each dispinterface and pass the new function
to create_automation_object.- */
+typedef struct {
- /*
* 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 */
- LPVOID funcInvoke;
+} AutomationObject;
You shouldn't need to expose the implementation details of AutomationObject outside of automation.c.
+/* This is the function that one needs to call to create an automation object. */ +extern HRESULT create_automation_object(MSIHANDLE msiHandle, IUnknown *pUnkOuter, LPVOID *ppObj, REFIID clsid, LPVOID funcInvoke);
It is dangerous to use LPVOID as the type to pass a function in.
+/* We need to expose these functions because our IActiveScriptSite calls it */ +extern HRESULT WINAPI LoadTypeInfo(IDispatch *iface, ITypeInfo **pptinfo, REFIID clsid, LCID lcid); +extern HRESULT WINAPI SessionImpl_Invoke(AutomationObject*,DISPID,REFIID,LCID,WORD,DISPPARAMS*,VARIANT*,EXCEPINFO*,UINT*);
+/* Disp Ids
- (not complete, look in msiserver.idl to add more) */
+typedef enum {
- RecordDispId_StringData=1,
- RecordDispId_IntegerData,
- RecordDispId_SetStream,
- RecordDispId_ReadStream,
- RecordDispId_FieldCount,
- RecordDispId_IsNull,
- RecordDispId_DataSize,
- RecordDispId_ClearData,
- RecordDispId_FormatText
+} RecordDispId;
+typedef enum {
- ViewDispId_Execute=1,
- ViewDispId_Fetch,
- ViewDispId_Modify,
- ViewDispId_Close,
- ViewDispId_ColumnInfo,
- ViewDispId_GetError
+} ViewDispId;
+typedef enum {
- DatabaseDispId_DatabaseState=1,
- DatabaseDispId_SummaryInformation,
- DatabaseDispId_OpenView,
- DatabaseDispId_Commit,
- DatabaseDispId_PrimaryKeys,
- DatabaseDispId_Import,
- DatabaseDispId_Export,
- DatabaseDispId_Merge,
- DatabaseDispId_GenerateTransform,
- DatabaseDispId_ApplyTransform,
- DatabaseDispId_EnableUIPreview,
- DatabaseDispId_TablePersistent,
- DatabaseDispId_CreateTransformSummaryInfo
+} DatabaseDispId;
+typedef enum {
- SessionDispId_Installer=1,
- SessionDispId_Property,
- SessionDispId_Language,
- SessionDispId_Mode,
- SessionDispId_Database,
- SessionDispId_SourcePath,
- SessionDispId_TargetPath,
- SessionDispId_DoAction,
- SessionDispId_Sequence,
- SessionDispId_EvaluateCondition,
- SessionDispId_FormatRecord,
- SessionDispId_Message,
- SessionDispId_FeatureCurrentState,
- SessionDispId_FeatureRequestState,
- SessionDispId_FeatureValidStates,
- SessionDispId_FeatureCost,
- SessionDispId_ComponentCurrentState,
- SessionDispId_ComponentRequestState,
- SessionDispId_SetInstallLevel,
- SessionDispId_VerifyDiskSpace,
- SessionDispId_ProductProperty,
- SessionDispId_FeatureInfo,
- SessionDispId_ComponentCost
+} SessionDispId;
See dlls/oleaut32/tests/tmarshal_dispids.h and dlls/oleaut32/tests/tmarshal.idl for how to use DISPIDs properly.
+typedef struct {
- IActiveScriptSite lpVtbl;
- AutomationObject *session;
You don't actually access this as anything other than an IDispatch object, so you might as well change to "IDispatch *session".
+LPCWSTR read_script_from_file(LPCWSTR szFile, INT type)
Should be static.
+/* JScript or VBScript? */ +LPCWSTR progid_from_type(INT type)
Should also be static.
+/*
- Call a script. This is our meat and potatoes.
- Currently, since the function is relatively new, it will always end up returning S_OK.
Think of it like a bonus feature, we can run the script - great. If we have a problem,
we are no worse off than if this function had not been called.- */
+DWORD call_script(MSIHANDLE hPackage, INT type, LPCWSTR filename, LPCWSTR function, LPCWSTR action) +{
- LPCWSTR script = NULL, progId = NULL;
- HRESULT hr;
- IActiveScript *iActiveScript = NULL;
- IActiveScriptParse *iActiveScriptParse = NULL;
I haven't seen this variable notation style before. In Hungarian notation "p" is used instead of "i" to show a pointer to an object exposing the interface declared.
Other comments sound good, I will make appropriate changes. I think I will make patch 2 the scripting functions with a dummy call_script that just does a TRACE, and then patch 3 will apply automation and scripting.
Misha