[PATCH 2/2] wmp: Implement IConnectionPoint[Container] and add _WMPOCXEvents

[Jacek Caban]

On 02/09/2018 04:21 PM, Jacek Caban wrote:
> Hi Anton,
>
> On 02/07/2018 09:24 PM, Anton Romanov wrote:
>> +    while(cConnections--) {
>> +        while(This->iter < This->cp->sinks_size && !This->cp->sinks[This->iter])
>> +            This->iter++;
>> +        if(This->iter == This->cp->sinks_size)
>> +            break;
>> +
>> +        pgcd[cnt].pUnk = (IUnknown*)This->cp->sinks[This->iter];
>
> I think you're missing IUnknown_AddRef() here.
>
>> +void call_sink(ConnectionPoint *This, DISPID dispid, DISPPARAMS *dispparams)
>> +{
>> +    DWORD i;
>> +
>> +    for(i=0; i<This->sinks_size; i++) {
>> +        if(This->sinks[i])
>> +            IDispatch_Invoke(This->sinks[i], dispid, &IID_NULL, LOCALE_SYSTEM_DEFAULT,
>> +                             DISPATCH_METHOD, dispparams, NULL, NULL, NULL);
>> +    }
>> +}
>
> You don't use it in this patch, so it's a dead code. Please remove it
> from this patch (and introduce it with a patch that uses it).
>
>>  HRESULT WINAPI WMPFactory_CreateInstance(IClassFactory *iface, IUnknown *outer,
>>          REFIID riid, void **ppv)
>>  {
>> @@ -941,22 +894,25 @@ HRESULT WINAPI WMPFactory_CreateInstance(IClassFactory *iface, IUnknown *outer,
>>      wmp->IProvideClassInfo2_iface.lpVtbl = &ProvideClassInfo2Vtbl;
>>      wmp->IPersistStreamInit_iface.lpVtbl = &PersistStreamInitVtbl;
>>      wmp->IOleInPlaceObjectWindowless_iface.lpVtbl = &OleInPlaceObjectWindowlessVtbl;
>> -    wmp->IConnectionPointContainer_iface.lpVtbl = &ConnectionPointContainerVtbl;
>>      wmp->IOleControl_iface.lpVtbl = &OleControlVtbl;
>>  
>>      wmp->ref = 1;
>>  
>> -    init_player_ifaces(wmp);
>> -
>> -    hdc = GetDC(0);
>> -    dpi_x = GetDeviceCaps(hdc, LOGPIXELSX);
>> -    dpi_y = GetDeviceCaps(hdc, LOGPIXELSY);
>> -    ReleaseDC(0, hdc);
>> +    hres = init_player(wmp);
>> +    if (hres == S_OK) {
>> +        ConnectionPointContainer_Init(wmp);
>> +        hdc = GetDC(0);
>> +        dpi_x = GetDeviceCaps(hdc, LOGPIXELSX);
>> +        dpi_y = GetDeviceCaps(hdc, LOGPIXELSY);
>> +        ReleaseDC(0, hdc);
>>  
>> -    wmp->extent.cx = MulDiv(192, 2540, dpi_x);
>> -    wmp->extent.cy = MulDiv(192, 2540, dpi_y);
>> +        wmp->extent.cx = MulDiv(192, 2540, dpi_x);
>> +        wmp->extent.cy = MulDiv(192, 2540, dpi_y);
>>  
>> -    hres = IOleObject_QueryInterface(&wmp->IOleObject_iface, riid, ppv);
>> -    IOleObject_Release(&wmp->IOleObject_iface);
>> +        hres = IOleObject_QueryInterface(&wmp->IOleObject_iface, riid, ppv);
>> +        IOleObject_Release(&wmp->IOleObject_iface);
>> +    }
>> +    if(hres != S_OK)
>> +        destroy_player(wmp);
>>      return hres;
>
> Error handling looks wrong here. If QueryInterface() fails, you'd both
> release wmp and call destroy_wmp. Also init_player can't fail, so it
> should return void.
>
> Thanks,
> Jacek


Also events.c is missing license header.


Thanks,

Jacek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20180209/0f202964/attachment-0001.html>