Signed-off-by: Sven Baars sven.wine@gmail.com --- dlls/strmbase/mediatype.c | 14 +++++++++++--- dlls/strmbase/renderer.c | 1 + 2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/dlls/strmbase/mediatype.c b/dlls/strmbase/mediatype.c index 91c0ddc60a..dd5773f5de 100644 --- a/dlls/strmbase/mediatype.c +++ b/dlls/strmbase/mediatype.c @@ -254,6 +254,7 @@ static HRESULT WINAPI IEnumMediaTypesImpl_Skip(IEnumMediaTypes * iface, ULONG cM static HRESULT WINAPI IEnumMediaTypesImpl_Reset(IEnumMediaTypes * iface) { ULONG i; + HRESULT hr; AM_MEDIA_TYPE amt; IEnumMediaTypesImpl *This = impl_from_IEnumMediaTypes(iface);
@@ -264,14 +265,21 @@ static HRESULT WINAPI IEnumMediaTypesImpl_Reset(IEnumMediaTypes * iface) CoTaskMemFree(This->enumMediaDetails.pMediaTypes);
i = 0; - while (This->enumMediaFunction(This->basePin, i, &amt) == S_OK) i++; + while (This->enumMediaFunction(This->basePin, i, &amt) == S_OK) + { + FreeMediaType(&amt); + 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))) + This->enumMediaFunction(This->basePin, i, &amt); + hr = CopyMediaType(&This->enumMediaDetails.pMediaTypes[i], &amt); + FreeMediaType(&amt); + + if (FAILED(hr)) { while (i--) FreeMediaType(&This->enumMediaDetails.pMediaTypes[i]); diff --git a/dlls/strmbase/renderer.c b/dlls/strmbase/renderer.c index c06661ce73..3d5e4b5e7d 100644 --- a/dlls/strmbase/renderer.c +++ b/dlls/strmbase/renderer.c @@ -346,6 +346,7 @@ HRESULT WINAPI BaseRendererImpl_Receive(BaseRenderer *This, IMediaSample * pSamp { return VFW_E_TYPE_NOT_ACCEPTED; } + DeleteMediaType(pmt); }
This->pMediaSample = pSample;
Signed-off-by: Sven Baars sven.wine@gmail.com --- dlls/wsdapi/soap.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c index 2ecc01313f..aa8d15f4c5 100644 --- a/dlls/wsdapi/soap.c +++ b/dlls/wsdapi/soap.c @@ -1426,6 +1426,9 @@ static HRESULT ws_element_to_wsdxml_element(WS_XML_READER *reader, IWSDXMLContex if (FAILED(ret)) goto cleanup; WSDXMLAddChild(cur_element, element);
+ WSDFreeLinkedMemory(name); + name = NULL; + cur_wsd_attrib = NULL;
/* Add attributes */ @@ -1462,6 +1465,9 @@ static HRESULT ws_element_to_wsdxml_element(WS_XML_READER *reader, IWSDXMLContex new_wsd_attrib->Element = cur_element; new_wsd_attrib->Next = NULL;
+ WSDAttachLinkedMemory(new_wsd_attrib, name); + name = NULL; + if (cur_wsd_attrib == NULL) element->FirstAttribute = new_wsd_attrib; else @@ -1524,6 +1530,7 @@ outofmemory: cleanup: /* Free uri and element_name if applicable */ WSDFreeLinkedMemory(uri); + WSDFreeLinkedMemory(name); return ret; }
@@ -1712,10 +1719,10 @@ HRESULT read_message(IWSDiscoveryPublisherImpl *impl, const char *xml, int xml_l IWSDXMLContext *context = NULL; WS_XML_STRING *soap_uri = NULL; const WS_XML_NODE *node; - WS_XML_READER *reader; + WS_XML_READER *reader = NULL; LPCWSTR value = NULL; LPWSTR uri, prefix; - WS_HEAP *heap; + WS_HEAP *heap = NULL; HRESULT ret; int i;
@@ -1945,6 +1952,8 @@ cleanup: free_xml_string(soap_uri); WSDFreeLinkedMemory(soap_msg); if (context != NULL) IWSDXMLContext_Release(context); + if (reader != NULL) WsFreeReader(reader); + if (heap != NULL) WsFreeHeap(heap);
return ret; }
Signed-off-by: Sven Baars sven.wine@gmail.com --- This was added in 5694825ae3c3a581976716a62e1b2d1fb54e5674, but the corrected test shows this may not be correct. Maybe Vincent still knows why this was done?
dlls/gdiplus/font.c | 20 ++++---------------- dlls/gdiplus/tests/font.c | 14 +++++++++----- 2 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 5e6aa5430f..cbe7859fe7 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -1576,7 +1576,6 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList( GpFontFamily* gpfamilies[], INT* numFound) { INT i; - GpStatus stat=Ok;
TRACE("%p, %d, %p, %p\n", fontCollection, numSought, gpfamilies, numFound);
@@ -1585,24 +1584,13 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList(
memset(gpfamilies, 0, sizeof(*gpfamilies) * numSought);
- for (i = 0; i < numSought && i < fontCollection->count && stat == Ok; i++) + for (i = 0; i < numSought && i < fontCollection->count; i++) { - stat = GdipCloneFontFamily(fontCollection->FontFamilies[i], &gpfamilies[i]); + gpfamilies[i] = fontCollection->FontFamilies[i]; } + *numFound = i;
- if (stat == Ok) - *numFound = i; - else - { - int numToFree=i; - for (i=0; i<numToFree; i++) - { - GdipDeleteFontFamily(gpfamilies[i]); - gpfamilies[i] = NULL; - } - } - - return stat; + return Ok; }
void free_installed_fonts(void) diff --git a/dlls/gdiplus/tests/font.c b/dlls/gdiplus/tests/font.c index 03e7a04e08..f91418d806 100644 --- a/dlls/gdiplus/tests/font.c +++ b/dlls/gdiplus/tests/font.c @@ -1187,7 +1187,7 @@ todo_wine
static void test_GdipGetFontCollectionFamilyList(void) { - GpFontFamily *family, *family2; + GpFontFamily *family, *family2, **families; GpFontCollection *collection; INT found, count; GpStatus status; @@ -1228,15 +1228,19 @@ static void test_GdipGetFontCollectionFamilyList(void) ok(found == 1, "Unexpected list count %d.\n", found); ok(family != NULL, "Expected family instance.\n");
- family = NULL; + family2 = NULL; found = 0; status = GdipGetFontCollectionFamilyList(collection, 1, &family2, &found); ok(status == Ok, "Failed to get family list, status %d.\n", status); ok(found == 1, "Unexpected list count %d.\n", found); - ok(family2 != family, "Unexpected family instance.\n"); + ok(family2 == family, "Unexpected family instance.\n");
- GdipDeleteFontFamily(family); - GdipDeleteFontFamily(family2); + families = GdipAlloc((count + 1) * sizeof(*families)); + found = 0; + status = GdipGetFontCollectionFamilyList(collection, count + 1, families, &found); + ok(status == Ok, "Failed to get family list, status %d.\n", status); + ok(found == count, "Unexpected list count %d, extected %d.\n", found, count); + GdipFree(families); }
static void test_GdipGetFontCollectionFamilyCount(void)
On 1/24/19 1:59 PM, Sven Baars wrote:
Signed-off-by: Sven Baars sven.wine@gmail.com
This was added in 5694825ae3c3a581976716a62e1b2d1fb54e5674, but the corrected test shows this may not be correct. Maybe Vincent still knows why this was done?
dlls/gdiplus/font.c | 20 ++++---------------- dlls/gdiplus/tests/font.c | 14 +++++++++----- 2 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 5e6aa5430f..cbe7859fe7 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -1576,7 +1576,6 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList( GpFontFamily* gpfamilies[], INT* numFound) { INT i;
GpStatus stat=Ok;
TRACE("%p, %d, %p, %p\n", fontCollection, numSought, gpfamilies, numFound);
@@ -1585,24 +1584,13 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList(
memset(gpfamilies, 0, sizeof(*gpfamilies) * numSought);
- for (i = 0; i < numSought && i < fontCollection->count && stat == Ok; i++)
- for (i = 0; i < numSought && i < fontCollection->count; i++) {
stat = GdipCloneFontFamily(fontCollection->FontFamilies[i], &gpfamilies[i]);
gpfamilies[i] = fontCollection->FontFamilies[i]; }- *numFound = i;
- if (stat == Ok)
*numFound = i;- else
- {
int numToFree=i;for (i=0; i<numToFree; i++){GdipDeleteFontFamily(gpfamilies[i]);gpfamilies[i] = NULL;}- }
- return stat;
- return Ok; }
Last time I looked at this one, I remember that it needed to be reference-counted.
Test path for that is to compare instance returned from GdipGetFamily() to what you get from collection.
void free_installed_fonts(void) diff --git a/dlls/gdiplus/tests/font.c b/dlls/gdiplus/tests/font.c index 03e7a04e08..f91418d806 100644 --- a/dlls/gdiplus/tests/font.c +++ b/dlls/gdiplus/tests/font.c @@ -1187,7 +1187,7 @@ todo_wine
static void test_GdipGetFontCollectionFamilyList(void) {
- GpFontFamily *family, *family2;
- GpFontFamily *family, *family2, **families; GpFontCollection *collection; INT found, count; GpStatus status;
@@ -1228,15 +1228,19 @@ static void test_GdipGetFontCollectionFamilyList(void) ok(found == 1, "Unexpected list count %d.\n", found); ok(family != NULL, "Expected family instance.\n");
- family = NULL;
- family2 = NULL; found = 0; status = GdipGetFontCollectionFamilyList(collection, 1, &family2, &found); ok(status == Ok, "Failed to get family list, status %d.\n", status); ok(found == 1, "Unexpected list count %d.\n", found);
- ok(family2 != family, "Unexpected family instance.\n");
- ok(family2 == family, "Unexpected family instance.\n");
- GdipDeleteFontFamily(family);
- GdipDeleteFontFamily(family2);
families = GdipAlloc((count + 1) * sizeof(*families));
found = 0;
status = GdipGetFontCollectionFamilyList(collection, count + 1, families, &found);
ok(status == Ok, "Failed to get family list, status %d.\n", status);
ok(found == count, "Unexpected list count %d, extected %d.\n", found, count);
GdipFree(families); }
static void test_GdipGetFontCollectionFamilyCount(void)
On 24-01-19 12:12, Nikolay Sivov wrote:
On 1/24/19 1:59 PM, Sven Baars wrote:
Signed-off-by: Sven Baars sven.wine@gmail.com
This was added in 5694825ae3c3a581976716a62e1b2d1fb54e5674, but the corrected test shows this may not be correct. Maybe Vincent still knows why this was done?
dlls/gdiplus/font.c | 20 ++++---------------- dlls/gdiplus/tests/font.c | 14 +++++++++----- 2 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index 5e6aa5430f..cbe7859fe7 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -1576,7 +1576,6 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList( GpFontFamily* gpfamilies[], INT* numFound) { INT i; - GpStatus stat=Ok; TRACE("%p, %d, %p, %p\n", fontCollection, numSought, gpfamilies, numFound); @@ -1585,24 +1584,13 @@ GpStatus WINGDIPAPI GdipGetFontCollectionFamilyList( memset(gpfamilies, 0, sizeof(*gpfamilies) * numSought); - for (i = 0; i < numSought && i < fontCollection->count && stat == Ok; i++) + for (i = 0; i < numSought && i < fontCollection->count; i++) { - stat = GdipCloneFontFamily(fontCollection->FontFamilies[i], &gpfamilies[i]); + gpfamilies[i] = fontCollection->FontFamilies[i]; } + *numFound = i; - if (stat == Ok) - *numFound = i; - else - { - int numToFree=i; - for (i=0; i<numToFree; i++) - { - GdipDeleteFontFamily(gpfamilies[i]); - gpfamilies[i] = NULL; - } - }
- return stat; + return Ok; }
Last time I looked at this one, I remember that it needed to be reference-counted.
Test path for that is to compare instance returned from GdipGetFamily() to what you get from collection.
Apparently the font families are indeed not cloned for the fonts themselves:
https://testbot.winehq.org/JobDetails.pl?Key=46590
but using GdipDeleteFontFamily on the font family that is returned from GdipGetFontCollectionFamilyList crashes on Windows as can be seen here
https://testbot.winehq.org/JobDetails.pl?Key=46598
whereas this succeeds (removing only the topmost test)
https://testbot.winehq.org/JobDetails.pl?Key=46594
so I don't think it is refcounted there (or the refcount is not increased). At least I would assume that GdipDeleteFontFamily reduced the refcount if it worked like that. That or GdipNewPrivateFontCollection is just bugged since deleting the font family does work in the other tests...
I think I've seen applications delete font families enumerated from the installed font collection.
Maybe there's no reference count, and installed fonts have a single family object forever (or until GdiplusShutdown).
On 24-01-19 16:25, Vincent Povirk wrote:
I think I've seen applications delete font families enumerated from the installed font collection.
Maybe there's no reference count, and installed fonts have a single family object forever (or until GdiplusShutdown).
Should I just submit the improved test with the todo_wines and leave the memory leaks then?
Should I just submit the improved test with the todo_wines and leave the memory leaks then?
Yes, that sounds useful.
Signed-off-by: Sven Baars sven.wine@gmail.com --- dlls/gdiplus/image.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 2e844cfcc5..8a136eff09 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -3570,7 +3570,11 @@ static GpStatus initialize_decoder_wic(IStream *stream, REFGUID container, IWICB if (FAILED(hr)) return hresult_to_status(hr);
hr = IWICBitmapDecoder_Initialize(*decoder, stream, WICDecodeMetadataCacheOnLoad); - if (FAILED(hr)) return hresult_to_status(hr); + if (FAILED(hr)) + { + IWICBitmapDecoder_Release(*decoder); + return hresult_to_status(hr); + } return Ok; }
Signed-off-by: Vincent Povirk vincent@codeweavers.com
Signed-off-by: Sven Baars sven.wine@gmail.com --- dlls/windowscodecs/pngformat.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/windowscodecs/pngformat.c b/dlls/windowscodecs/pngformat.c index 7050c95121..faffeae6ae 100644 --- a/dlls/windowscodecs/pngformat.c +++ b/dlls/windowscodecs/pngformat.c @@ -620,7 +620,6 @@ static HRESULT WINAPI PngDecoder_Initialize(IWICBitmapDecoder *iface, IStream *p if (setjmp(jmpbuf)) { ppng_destroy_read_struct(&This->png_ptr, &This->info_ptr, &This->end_info); - HeapFree(GetProcessHeap(), 0, row_pointers); This->png_ptr = NULL; hr = WINCODEC_ERR_UNKNOWNIMAGEFORMAT; goto end; @@ -816,6 +815,9 @@ static HRESULT WINAPI PngDecoder_Initialize(IWICBitmapDecoder *iface, IStream *p end: LeaveCriticalSection(&This->lock);
+ if (row_pointers) + HeapFree(GetProcessHeap(), 0, row_pointers); + return hr; }
Signed-off-by: Vincent Povirk vincent@codeweavers.com
Sven Baars sven.wine@gmail.com writes:
for (i = 0; i < This->enumMediaDetails.cMediaTypes; i++) {
This->enumMediaFunction(This->basePin, i,&amt);if (FAILED(CopyMediaType(&This->enumMediaDetails.pMediaTypes[i], &amt)))
This->enumMediaFunction(This->basePin, i, &amt);hr = CopyMediaType(&This->enumMediaDetails.pMediaTypes[i], &amt);FreeMediaType(&amt);
The extra copy doesn't seem necessary, you could fetch directly into the destination array.
On 29-01-19 19:21, Alexandre Julliard wrote:
Sven Baars sven.wine@gmail.com writes:
for (i = 0; i < This->enumMediaDetails.cMediaTypes; i++) {
This->enumMediaFunction(This->basePin, i,&amt);if (FAILED(CopyMediaType(&This->enumMediaDetails.pMediaTypes[i], &amt)))
This->enumMediaFunction(This->basePin, i, &amt);hr = CopyMediaType(&This->enumMediaDetails.pMediaTypes[i], &amt);FreeMediaType(&amt);The extra copy doesn't seem necessary, you could fetch directly into the destination array.
That seems to work indeed. I will send a new patch. Thanks!
-
Alexandre Julliard -
Nikolay Sivov -
Sven Baars -
Vincent Povirk