Re: [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.