Re: [PATCH] amstream: Initialize correctly AM_MEDIA_TYPE struct.

[Christian Costa]

Le 16/11/2012 00:24, Michael Stefaniuc a écrit :
Hello Christian,
On 11/15/2012 09:43 AM, Christian Costa wrote:
Fixes bug 32185.
---
  dlls/amstream/mediastreamfilter.c |    2 ++
  1 file changed, 2 insertions(+)

diff --git a/dlls/amstream/mediastreamfilter.c 
b/dlls/amstream/mediastreamfilter.c
index 9b2bd8e..d5388e5 100644
--- a/dlls/amstream/mediastreamfilter.c
+++ b/dlls/amstream/mediastreamfilter.c
@@ -132,6 +132,8 @@ static HRESULT WINAPI BasePinImp_GetMediaType(BasePin 
*This, int index, AM_MEDIA
      MSPID purpose_id;
      int i;
  
+    ZeroMemory(amt, sizeof(*amt));
+
this looks "odd". There is no check if amt is NULL, at least that's what
caught my attention. So I've looked around and stumbled upon the
AM_MEDIA_TYPE structure documentation
http://msdn.microsoft.com/en-us/library/windows/desktop/dd373477%28v=vs.85%29.aspx
   "pUnk
        Not used. Set to NULL."

Setting that to NULL in amstream's BasePinImp_GetMediaType() fixes the
crash for me. But who's responsibility is to set / check that? That
function or the caller aka test_media_streams() ?

bye
        michael



This is an helper function called by winestrmbase. It's internal. There 
is no need to do a null check.
The crash depends of the stack content.

AM_MEDIA_TYPE amt;
while (This->enumMediaFunction(This->basePin, i,&amt) == S_OK) i++;

Where supposed to fill all the structure fields. Off course we can make 
winestrmbase function zero it before calling the helper.

Here is the entire function

static HRESULT WINAPI IEnumMediaTypesImpl_Reset(IEnumMediaTypes * iface)
{
    ULONG i;
    AM_MEDIA_TYPE amt;
    IEnumMediaTypesImpl *This = impl_from_IEnumMediaTypes(iface);

    TRACE("()\n");

    for (i = 0; i < This->enumMediaDetails.cMediaTypes; i++)
        if (This->enumMediaDetails.pMediaTypes[i].pbFormat)
CoTaskMemFree(This->enumMediaDetails.pMediaTypes[i].pbFormat);
    CoTaskMemFree(This->enumMediaDetails.pMediaTypes);

    i = 0;
    while (This->enumMediaFunction(This->basePin, i,&amt) == S_OK) i++;

    This->enumMediaDetails.cMediaTypes = i;
    This->enumMediaDetails.pMediaTypes = 
CoTaskMemAlloc(sizeof(AM_MEDIA_TYPE) * i);
    for (i = 0; i < This->enumMediaDetails.cMediaTypes; i++)
    {
        This->enumMediaFunction(This->basePin, i,&amt);
        if 
(FAILED(CopyMediaType(&This->enumMediaDetails.pMediaTypes[i], &amt)))
        {
           while (i--)
CoTaskMemFree(This->enumMediaDetails.pMediaTypes[i].pbFormat);
           CoTaskMemFree(This->enumMediaDetails.pMediaTypes);
           return E_OUTOFMEMORY;
        }
    }

    This->currentVersion = This->mediaVersionFunction(This->basePin);
    This->uIndex = 0;

    return S_OK;
}