Re: [1/2] windowscodecs: Add a test for CreateBitmapFromHICON.
On 1/11/2013 14:39, Dmitry Timoshkov wrote:
+ icon = CreateIconIndirect(&info); + ok(icon != 0, "CreateIconIndirect failed\n"); + DeleteObject(info.hbmColor); + DeleteObject(info.hbmMask); + + hr = IWICImagingFactory_CreateBitmapFromHICON(factory, icon, &bitmap); + ok(hr == S_OK, "CreateBitmapFromHICON error %#x\n", hr); + + IWICBitmap_GetPixelFormat(bitmap, &format); + ok(IsEqualGUID(&format, &GUID_WICPixelFormat32bppBGRA), + "unexpected pixel format %s\n", debugstr_guid(&format)); + + IWICBitmap_GetSize(bitmap, &width, &height); + ok(hr == S_OK, "IWICBitmap_GetSize error %#x\n", hr); + ok(width == 16, "expected 16, got %u\n", width); + ok(height == 16, "expected 16, got %u\n", height); + + IWICBitmap_Release(bitmap); +} Who is responsible for freeing icon handle? I guess it should be done here cause your implementation doesn't free it either.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
Who is responsible for freeing icon handle? I guess it should be done here cause your implementation doesn't free it either.
You are right, DestroyIcon() is missing at the end of the test. Consider that as a feature, the tests are not supposed to free all the allocated resources, it's a waste of time :) -- Dmitry.
On 1/11/2013 14:58, Dmitry Timoshkov wrote:
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
Who is responsible for freeing icon handle? I guess it should be done here cause your implementation doesn't free it either. You are right, DestroyIcon() is missing at the end of the test. Consider that as a feature, the tests are not supposed to free all the allocated resources, it's a waste of time :)
Fixing leaks like that is not optional. One reason to do this is to make valgrind output clean so it contains only real leaks in code. Another reason is that test could serve as an example of API usage.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
Who is responsible for freeing icon handle? I guess it should be done here cause your implementation doesn't free it either. You are right, DestroyIcon() is missing at the end of the test. Consider that as a feature, the tests are not supposed to free all the allocated resources, it's a waste of time :)
Fixing leaks like that is not optional. One reason to do this is to make valgrind output clean so it contains only real leaks in code. Another reason is that test could serve as an example of API usage.
I'd like to hear Alexandre's opinion on this, but freeing allocated memory in tests has never been a requirement, and Wine tests are not supposed to be a set of samples to learn windows programming. -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
Who is responsible for freeing icon handle? I guess it should be done here cause your implementation doesn't free it either. You are right, DestroyIcon() is missing at the end of the test. Consider that as a feature, the tests are not supposed to free all the allocated resources, it's a waste of time :)
Fixing leaks like that is not optional. One reason to do this is to make valgrind output clean so it contains only real leaks in code. Another reason is that test could serve as an example of API usage.
I'd like to hear Alexandre's opinion on this, but freeing allocated memory in tests has never been a requirement, and Wine tests are not supposed to be a set of samples to learn windows programming.
In that kind of test it's a good idea to destroy the object right after it's used, to catch cases where e.g. CreateBitmap would hold a reference to the icon instead of copying it or things like that. -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Dmitry Timoshkov -
Nikolay Sivov