[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: acmstream mem leak fix
From:       MichaelGuennewig () gmx ! de (=?iso-8859-1?q?Michael_G=FCnnewig?=)
Date:       2004-11-30 19:02:55
Message-ID: m3mzwzb1nk.fsf () harry ! guenne-net ! loc
[Download RAW message or body]

James Hawkins <truiken@gmail.com> writes:

> Unless I'm mistaken, we don't want to free allocated memory only when
> the call fails.

Sorry, but you are wrong. When you always free the memory you will run
into a segmentation fault when the application uses the returned
stream.

The problem is the same as with the icmstream patch you made. The
current implementation will work correctly in all cases although it doesn't
look very COM-like as Mike already pointed out for the icmstream patch.

The methods you analyzed are only called internally from
IClassFactory_fnCreateInstance in the factory.c file.

The memory block which is allocated by LocalAlloc in the
AVIFILE_CreateICMStream and AVIFILE_CreateACMStream is only returned
to the application if the IAVIStream_QueryInterface call doesn't
return an error.  So when you always free the memory block you will
most probably run into a segmentation fault.

The proposed patch from Mike for the icmstream.c file should be
adapted to this file when you don't like the LocalFree call there, but
I'm sure that then people will ask why we don't call LocalFree in
this method. So it would be perhaps fine when also some comments would
be added to prevent another discussion about what's wrong or correct.

When you don't understand the code feel free to write me as I'm the
one who has produced it.

  Regards

    Michael Günnewig


[prev in list] [next in list] [prev in thread] [next in thread]