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 <hans(a)codeweavers.com> 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.