Re: msi: Add full JScript/VBScript support and partial expandable OLE automation support. [PATCH 3/3]
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.
+ + 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. -- Rob Shearman
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
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.
+ + 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.
I have implemented all the changes you outlined (thanks a lot) in this and your other email, with the exception of the extra error checking in each of the individual Invoke functions as they are called from within AutomationObject::Invoke after the error checking already occurs. What do you think? Thanks Misha p.s. The 5th patch is not a part of my submission, but is just a hack I used to make the IDL file with the dispids using oleview. This patch will make oleview use the DISPID_PARENT_MEMBER notation for ids and will output the include file to standard output. Thought maybe it would be useful for someone.
participants (2)
-
Misha Koshelev -
Robert Shearman