[PATCH] dmime: IDirectMusicPerformance8_FreePMsg Release the unknown pointer.

[Michael Stefaniuc]

Hello Alistair,

On 12/19/19 7:59 AM, Alistair Leslie-Hughes wrote:
> I have another patch that causes this function to be called (faking a 
> set notification).  So, Yes it's currently a noop but it's small and not 
> going to effect anything.
if you need it for a patch that fixes a bug than sure, please resubmit
it as patch series with that patch during the freeze.

I've would have signed-off on the patch after reading the documentation
I've linked. But the timing wasn't right for a low risk but zero benefit
during the freeze patch.

> I happy for it to be differed for required.
I've had to create a 'dmdefer' branch anyway for my two outstanding
patches. So I've added your patch to that too.

thanks
bye
	michael


> 
> Alistair.
> 
> 
> On 19/12/19 7:55 am, Michael Stefaniuc wrote:
>> Hello Alistair,
>>
>> while this patch is technically correct
>> https://nam02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdoc.51windows.net%2FDirectx9_SDK%2Fhtm%2Fdmuspmsg.htm&data=02%7C01%7C%7C97391116c3fe4e82837508d783fc9a14%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637122993246323981&sdata=TnqEFSAm48rUdOM0NKD2qoQO%2BeZlVRVSysyk8enyBIE%3D&reserved=0
>> I'm wondering why it was triggered by bug #25728 (comments 34 and 35).
>>
>> I don't see a FreePMsg in the log
>> https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.winehq.org%2Fattachment.cgi%3Fid%3D65980&data=02%7C01%7C%7C97391116c3fe4e82837508d783fc9a14%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637122993246323981&sdata=HONREqoDqPoo2OBNHWXuFrqz9%2B4TMBOY%2FaYEnPLlFiE%3D&reserved=0
>> I also don't see a FreePMsg in any of the logs I have.
>> Nor in the attachments of all directx-dmusic bugs.
>>
>> Also we don't set punkUser anywhere in the whole Wine code
>>
>> wine$ git grep -n punkUser
>> dlls/dmime/performance.c:482:  if (pPMSG->punkUser)
>> dlls/dmime/performance.c:483:    IUnknown_Release(pPMSG->punkUser);
>> dlls/dmime/tests/performance.c:447:            ok(msg->punkUser != NULL,
>> "Unexpected NULL pointer\n");
>> dlls/dmime/tests/performance.c:448:            if (msg->punkUser) {
>> dlls/dmime/tests/performance.c:452:                hr =
>> IUnknown_QueryInterface(msg->punkUser, &IID_IDirectMusicSegmentState8,
>> (void**)&segmentstate);
>> include/dmusici.h:363:  IUnknown*          punkUser;
>>
>> While we seem to have a test that checks for non-NULL punkUser, that
>> test doesn't runs on Wine. Changing to ok(msg->punkUser == NULL, ...)
>> still passes on Wine.
>> That test does run on Windows though
>> https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftestbot.winehq.org%2FJobDetails.pl%3FKey%3D62304&data=02%7C01%7C%7C97391116c3fe4e82837508d783fc9a14%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637122993246323981&sdata=z9JNq9PcKC3HBYRVcRgOzcPjgseMBpzN%2FefiA6KU9j0%3D&reserved=0
>>
>>
>> So this patch is a no-op for now iff we allocate zero all the messages.
>> Else we might crash on uninitialized memory. So it does sound like a
>> "Deferred" to me.
>>
>>
>> bye
>> 	michael
>>
>>
>>
>> On 12/18/19 6:17 AM, Alistair Leslie-Hughes wrote:
>>> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair at hotmail.com>
>>> ---
>>>   dlls/dmime/performance.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c
>>> index 9dbb469f388..a63bd3b4517 100644
>>> --- a/dlls/dmime/performance.c
>>> +++ b/dlls/dmime/performance.c
>>> @@ -479,7 +479,9 @@ static HRESULT WINAPI IDirectMusicPerformance8Impl_FreePMsg(IDirectMusicPerforma
>>>     DMUS_ItemRemoveFromQueue( This, pItem );
>>>     LeaveCriticalSection(&This->safe);
>>>   
>>> -  /** TODO: see if we should Release the pItem->pMsg->punkUser and others Interfaces */
>>> +  if (pPMSG->punkUser)
>>> +    IUnknown_Release(pPMSG->punkUser);
>>> +
>>>     HeapFree(GetProcessHeap(), 0, pItem);
>>>     return S_OK;
>>>   }
>>>
>>