Re: [PATCH 1/7] msi/custom.c: unify thread handling of different custom action types (resend)

[Hans Leidekker]

On Tue, 2011-10-11 at 13:20 +0200, Bernhard Loos wrote:
+static HRESULT get_action_info( msi_custom_action_info *info, INT *type,
+                                BSTR *source, BSTR *target, BSTR *name,
+                                IWineMsiRemotePackage **package )
 {
-    IClassFactory *cf = NULL;
-    IWineMsiRemoteCustomAction *rca = NULL;
-    HRESULT r;
-
-    r = DllGetClassObject( &CLSID_WineMsiRemoteCustomAction,
-                           &IID_IClassFactory, (LPVOID *)&cf );
-    if (FAILED(r))
-    {
-        ERR("failed to get IClassFactory interface\n");
-        return ERROR_FUNCTION_FAILED;
-    }
-
-    r = IClassFactory_CreateInstance( cf, NULL, &IID_IWineMsiRemoteCustomAction, (LPVOID *)&rca );
-    if (FAILED(r))
-    {
-        ERR("failed to get IWineMsiRemoteCustomAction interface\n");
-        return ERROR_FUNCTION_FAILED;
-    }
-
-    r = IWineMsiRemoteCustomAction_GetActionInfo( rca, guid, type, handle, dll, funcname, package );
-    IWineMsiRemoteCustomAction_Release( rca );
-    if (FAILED(r))
-    {
-        ERR("GetActionInfo failed\n");
-        return ERROR_FUNCTION_FAILED;
+    IWineMsiRemotePackage *rp;
+    HRESULT res;
+    *source = *target = *name = NULL;
+    
+    res = create_msi_remote_package( NULL, (void **) &rp );
+    if (FAILED( res ))
+        return res;
+    res = IWineMsiRemotePackage_SetMsiHandle( rp, alloc_msihandle( &info->package->hdr ) );

You should check for failure from alloc_msihandle.

+    if (FAILED( res ))
+        goto error;
+        
+    *source  = SysAllocString( info->source );
+    *target  = SysAllocString( info->target );
+    *name    = SysAllocString( info->action );
+    if (!*source || !*target || !*name) {
+        res = E_OUTOFMEMORY;
+        goto error;
     }
-
+    
+    *package = rp;
+    *type    = info->type;
+    
     return ERROR_SUCCESS;

This should become S_OK. You are mixing up return types in more places.

[PATCH 1/7] msi/custom.c: unify thread handling of different custom action types (resend)

[Bernhard Loos]

On Tue, Oct 11, 2011 at 2:29 PM, Hans Leidekker  wrote:
> On Tue, 2011-10-11 at 13:20 +0200, Bernhard Loos wrote:
> +static HRESULT get_action_info( msi_custom_action_info *info, INT *type,
> - BSTR *source, BSTR *target, BSTR *name,
> - IWineMsiRemotePackage **package )
>
> {
> - IClassFactory *cf = NULL;
> - IWineMsiRemoteCustomAction *rca = NULL;
> - HRESULT r;
> -
> - r = DllGetClassObject( &CLSID_WineMsiRemoteCustomAction,
> - &IID_IClassFactory, (LPVOID *)&cf );
> - if (FAILED(r))
> - {
> - ERR("failed to get IClassFactory interface\n");
> - return ERROR_FUNCTION_FAILED;
> - }
> -
> - r = IClassFactory_CreateInstance( cf, NULL, &IID_IWineMsiRemoteCustomAction, (LPVOID *)&rca );
> - if (FAILED(r))
> - {
> - ERR("failed to get IWineMsiRemoteCustomAction interface\n");
> - return ERROR_FUNCTION_FAILED;
> - }
> -
> - r = IWineMsiRemoteCustomAction_GetActionInfo( rca, guid, type, handle, dll, funcname, package );
> - IWineMsiRemoteCustomAction_Release( rca );
> - if (FAILED(r))
> - {
> - ERR("GetActionInfo failed\n");
> - return ERROR_FUNCTION_FAILED;
> - IWineMsiRemotePackage *rp;
> - HRESULT res;
> - *source = *target = *name = NULL;
> -
> - res = create_msi_remote_package( NULL, (void **) &rp );
> - if (FAILED( res ))
> - return res;
> - res = IWineMsiRemotePackage_SetMsiHandle( rp, alloc_msihandle( &info->package->hdr ) );
>
> You should check for failure from alloc_msihandle.

In my defence, it currently also doesn't get checked, but I will add something.
> - if (FAILED( res ))
> - goto error;
> -
> - *source  = SysAllocString( info->source );
> - *target  = SysAllocString( info->target );
> - *name    = SysAllocString( info->action );
> - if (!*source || !*target || !*name) {
> - res = E_OUTOFMEMORY;
> - goto error;
>
> }
> -
> -
> - *package = rp;
> - *type    = info->type;
> -
>
> return ERROR_SUCCESS;
>
> This should become S_OK. You are mixing up return types in more places.