[PATCH v3 0/6] MR3302: gdiplus: Fix GdipGetPropertySize and GdipGetAllPropertyItems return values when no properties exist.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54748
-- v3: gdiplus: Fix GdipGetAllPropertyItems return value when no properties exist. gdiplus: Fix GdipGetPropertySize return value when no properties exist. gdiplus/tests: Pare down image property tests that are not run for Wine. gdiplus/tests: Use test context for test ran in a loop. gdiplus/tests: Add GIF to image properties tests. gdiplus/tests: Use consistent no-alternate value for image properties.
From: Jeff Smith whydoubt@gmail.com
--- dlls/gdiplus/tests/image.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index e32b4c81240..4931e3f313c 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3644,21 +3644,21 @@ static void test_image_properties(void) UINT image_size; ImageType image_type; UINT prop_count; - UINT prop_count2; /* if win7 behaves differently */ + UINT prop_count2; /* if win7+ behaves differently, else ~0 */ /* 1st property attributes */ UINT prop_size; - UINT prop_size2; /* if win7 behaves differently */ + UINT prop_size2; /* if win7+ behaves differently, else ~0 */ UINT prop_id; - UINT prop_id2; /* if win7 behaves differently */ + UINT prop_id2; /* if win7+ behaves differently, else ~0 */ INT palette_size; } td[] = { { pngimage, sizeof(pngimage), ImageTypeBitmap, 4, ~0, 1, 20, 0x5110, 0x132, 12 }, - { jpgimage, sizeof(jpgimage), ImageTypeBitmap, 2, ~0, 128, 0, 0x5090, 0x5091, 12 }, - { tiffimage, sizeof(tiffimage), ImageTypeBitmap, 16, 0, 4, 0, 0xfe, 0, 12 }, - { bmpimage, sizeof(bmpimage), ImageTypeBitmap, 0, 0, 0, 0, 0, 0, 16 }, - { wmfimage, sizeof(wmfimage), ImageTypeMetafile, 0, 0, 0, 0, 0, 0, -GenericError } + { jpgimage, sizeof(jpgimage), ImageTypeBitmap, 2, ~0, 128, ~0, 0x5090, 0x5091, 12 }, + { tiffimage, sizeof(tiffimage), ImageTypeBitmap, 16, ~0, 4, ~0, 0xfe, ~0, 12 }, + { bmpimage, sizeof(bmpimage), ImageTypeBitmap, 0, ~0, 0, ~0, 0, ~0, 16 }, + { wmfimage, sizeof(wmfimage), ImageTypeMetafile, 0, ~0, 0, ~0, 0, ~0, -GenericError } }; GpStatus status; GpImage *image; @@ -3704,7 +3704,7 @@ static void test_image_properties(void) status = GdipGetPropertyCount(image, &prop_count); ok(status == Ok, "%u: GdipGetPropertyCount error %d\n", i, status); todo_wine_if(td[i].image_data == pngimage || td[i].image_data == jpgimage) - ok(td[i].prop_count == prop_count || td[i].prop_count2 == prop_count, + ok(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count), " %u: expected property count %u or %u, got %u\n", i, td[i].prop_count, td[i].prop_count2, prop_count);
@@ -3729,7 +3729,7 @@ static void test_image_properties(void) expect(PropertyNotFound, status);
/* FIXME: remove once Wine is fixed */ - if (td[i].prop_count != prop_count) + if (!(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count))) { GdipDisposeImage(image); continue; @@ -3763,7 +3763,7 @@ static void test_image_properties(void) { expect(Ok, status); if (prop_count != 0) - ok(td[i].prop_id == prop_id[0] || td[i].prop_id2 == prop_id[0], + ok(td[i].prop_id == prop_id[0] || (td[i].prop_id2 != ~0 && td[i].prop_id2 == prop_id[0]), " %u: expected property id %#x or %#x, got %#lx\n", i, td[i].prop_id, td[i].prop_id2, prop_id[0]); } @@ -3781,9 +3781,9 @@ static void test_image_properties(void) ok(prop_size > sizeof(PropertyItem), "%u: got too small prop_size %u\n", i, prop_size); ok(td[i].prop_size + sizeof(PropertyItem) == prop_size || - td[i].prop_size2 + sizeof(PropertyItem) == prop_size, - " %u: expected property size %u or %u, got %u\n", - i, td[i].prop_size, td[i].prop_size2, prop_size); + (td[i].prop_size2 != ~0 && td[i].prop_size2 + sizeof(PropertyItem) == prop_size), + " %u: expected property size (%u or %u)+%u, got %u\n", + i, td[i].prop_size, td[i].prop_size2, (UINT) sizeof(PropertyItem), prop_size);
status = GdipGetPropertyItem(image, prop_id[0], 0, &item.data); ok(status == InvalidParameter || status == GenericError /* Win7 */,
From: Jeff Smith whydoubt@gmail.com
--- dlls/gdiplus/tests/image.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 4931e3f313c..9d916e3a6ff 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3658,6 +3658,7 @@ static void test_image_properties(void) { jpgimage, sizeof(jpgimage), ImageTypeBitmap, 2, ~0, 128, ~0, 0x5090, 0x5091, 12 }, { tiffimage, sizeof(tiffimage), ImageTypeBitmap, 16, ~0, 4, ~0, 0xfe, ~0, 12 }, { bmpimage, sizeof(bmpimage), ImageTypeBitmap, 0, ~0, 0, ~0, 0, ~0, 16 }, + { gifimage, sizeof(gifimage), ImageTypeBitmap, 1, 4, 4, ~0, 0x5100, ~0, 16 }, { wmfimage, sizeof(wmfimage), ImageTypeMetafile, 0, ~0, 0, ~0, 0, ~0, -GenericError } }; GpStatus status; @@ -3703,7 +3704,7 @@ static void test_image_properties(void)
status = GdipGetPropertyCount(image, &prop_count); ok(status == Ok, "%u: GdipGetPropertyCount error %d\n", i, status); - todo_wine_if(td[i].image_data == pngimage || td[i].image_data == jpgimage) + todo_wine_if(td[i].image_data == pngimage || td[i].image_data == jpgimage || td[i].image_data == gifimage) ok(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count), " %u: expected property count %u or %u, got %u\n", i, td[i].prop_count, td[i].prop_count2, prop_count);
From: Jeff Smith whydoubt@gmail.com
--- dlls/gdiplus/tests/image.c | 52 +++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 9d916e3a6ff..d4cbccd60d4 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3675,39 +3675,42 @@ static void test_image_properties(void)
for (i = 0; i < ARRAY_SIZE(td); i++) { + winetest_push_context("%u", i); + image = load_image(td[i].image_data, td[i].image_size, TRUE, FALSE); if (!image) { - trace("%u: failed to load image data\n", i); + trace("failed to load image data\n"); + winetest_pop_context(); continue; }
status = GdipGetImageType(image, &image_type); - ok(status == Ok, "%u: GdipGetImageType error %d\n", i, status); - ok(td[i].image_type == image_type, "%u: expected image_type %d, got %d\n", - i, td[i].image_type, image_type); + ok(status == Ok, "GdipGetImageType error %d\n", status); + ok(td[i].image_type == image_type, "expected image_type %d, got %d\n", + td[i].image_type, image_type);
palette_size = -1; status = GdipGetImagePaletteSize(image, &palette_size); if (td[i].palette_size >= 0) { - ok(status == Ok, "%u: GdipGetImagePaletteSize error %d\n", i, status); - ok(td[i].palette_size == palette_size, "%u: expected palette_size %d, got %d\n", - i, td[i].palette_size, palette_size); + ok(status == Ok, "GdipGetImagePaletteSize error %d\n", status); + ok(td[i].palette_size == palette_size, "expected palette_size %d, got %d\n", + td[i].palette_size, palette_size); } else { - ok(status == -td[i].palette_size, "%u: GdipGetImagePaletteSize returned %d\n", i, status); - ok(palette_size == 0, "%u: expected palette_size 0, got %d\n", - i, palette_size); + ok(status == -td[i].palette_size, "GdipGetImagePaletteSize returned %d\n", status); + ok(palette_size == 0, "expected palette_size 0, got %d\n", + palette_size); }
status = GdipGetPropertyCount(image, &prop_count); - ok(status == Ok, "%u: GdipGetPropertyCount error %d\n", i, status); + ok(status == Ok, "GdipGetPropertyCount error %d\n", status); todo_wine_if(td[i].image_data == pngimage || td[i].image_data == jpgimage || td[i].image_data == gifimage) ok(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count), - " %u: expected property count %u or %u, got %u\n", - i, td[i].prop_count, td[i].prop_count2, prop_count); + "expected property count %u or %u, got %u\n", + td[i].prop_count, td[i].prop_count2, prop_count);
status = GdipGetPropertyItemSize(NULL, 0, &prop_size); expect(InvalidParameter, status); @@ -3733,6 +3736,7 @@ static void test_image_properties(void) if (!(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count))) { GdipDisposeImage(image); + winetest_pop_context(); continue; }
@@ -3765,8 +3769,8 @@ static void test_image_properties(void) expect(Ok, status); if (prop_count != 0) ok(td[i].prop_id == prop_id[0] || (td[i].prop_id2 != ~0 && td[i].prop_id2 == prop_id[0]), - " %u: expected property id %#x or %#x, got %#lx\n", - i, td[i].prop_id, td[i].prop_id2, prop_id[0]); + "expected property id %#x or %#x, got %#lx\n", + td[i].prop_id, td[i].prop_id2, prop_id[0]); }
if (status == Ok) @@ -3779,30 +3783,32 @@ static void test_image_properties(void) expect(Ok, status);
assert(sizeof(item) >= prop_size); - ok(prop_size > sizeof(PropertyItem), "%u: got too small prop_size %u\n", - i, prop_size); + ok(prop_size > sizeof(PropertyItem), "got too small prop_size %u\n", + prop_size); ok(td[i].prop_size + sizeof(PropertyItem) == prop_size || (td[i].prop_size2 != ~0 && td[i].prop_size2 + sizeof(PropertyItem) == prop_size), - " %u: expected property size (%u or %u)+%u, got %u\n", - i, td[i].prop_size, td[i].prop_size2, (UINT) sizeof(PropertyItem), prop_size); + "expected property size (%u or %u)+%u, got %u\n", + td[i].prop_size, td[i].prop_size2, (UINT) sizeof(PropertyItem), prop_size);
status = GdipGetPropertyItem(image, prop_id[0], 0, &item.data); ok(status == InvalidParameter || status == GenericError /* Win7 */, - "%u: expected InvalidParameter, got %d\n", i, status); + "expected InvalidParameter, got %d\n", status); status = GdipGetPropertyItem(image, prop_id[0], prop_size - 1, &item.data); ok(status == InvalidParameter || status == GenericError /* Win7 */, - "%u: expected InvalidParameter, got %d\n", i, status); + "expected InvalidParameter, got %d\n", status); status = GdipGetPropertyItem(image, prop_id[0], prop_size + 1, &item.data); ok(status == InvalidParameter || status == GenericError /* Win7 */, - "%u: expected InvalidParameter, got %d\n", i, status); + "expected InvalidParameter, got %d\n", status); status = GdipGetPropertyItem(image, prop_id[0], prop_size, &item.data); expect(Ok, status); ok(prop_id[0] == item.data.id, - "%u: expected property id %#lx, got %#lx\n", i, prop_id[0], item.data.id); + "expected property id %#lx, got %#lx\n", prop_id[0], item.data.id); } }
GdipDisposeImage(image); + + winetest_pop_context(); } }
From: Jeff Smith whydoubt@gmail.com
--- dlls/gdiplus/tests/image.c | 100 +++++++++++++++---------------------- 1 file changed, 40 insertions(+), 60 deletions(-)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index d4cbccd60d4..27b6bc9b2de 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3732,78 +3732,58 @@ static void test_image_properties(void) else expect(PropertyNotFound, status);
- /* FIXME: remove once Wine is fixed */ - if (!(td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count))) - { - GdipDisposeImage(image); - winetest_pop_context(); - continue; - } - status = GdipGetPropertyIdList(NULL, prop_count, prop_id); expect(InvalidParameter, status); status = GdipGetPropertyIdList(image, prop_count, NULL); expect(InvalidParameter, status); - status = GdipGetPropertyIdList(image, 0, prop_id); - if (image_type == ImageTypeMetafile) - expect(NotImplemented, status); - else if (prop_count == 0) - expect(Ok, status); - else - expect(InvalidParameter, status); + expected = (image_type == ImageTypeMetafile) ? NotImplemented : InvalidParameter; status = GdipGetPropertyIdList(image, prop_count - 1, prop_id); - if (image_type == ImageTypeMetafile) - expect(NotImplemented, status); - else - expect(InvalidParameter, status); + expect(expected, status); status = GdipGetPropertyIdList(image, prop_count + 1, prop_id); - if (image_type == ImageTypeMetafile) - expect(NotImplemented, status); - else - expect(InvalidParameter, status); + expect(expected, status); + if (image_type != ImageTypeMetafile && prop_count == 0) + expected = Ok; + status = GdipGetPropertyIdList(image, 0, prop_id); + expect(expected, status); + expected = (image_type == ImageTypeMetafile) ? NotImplemented : Ok; status = GdipGetPropertyIdList(image, prop_count, prop_id); + expect(expected, status); + + status = GdipGetPropertyItemSize(image, prop_id[0], &prop_size); if (image_type == ImageTypeMetafile) expect(NotImplemented, status); - else + else if (prop_count == 0) + expect(PropertyNotFound, status); + /* FIXME: remove condition once Wine is fixed, i.e. this should just be an else */ + else if (td[i].prop_count == prop_count || (td[i].prop_count2 != ~0 && td[i].prop_count2 == prop_count)) { - expect(Ok, status); - if (prop_count != 0) - ok(td[i].prop_id == prop_id[0] || (td[i].prop_id2 != ~0 && td[i].prop_id2 == prop_id[0]), - "expected property id %#x or %#x, got %#lx\n", - td[i].prop_id, td[i].prop_id2, prop_id[0]); - } + ok(td[i].prop_id == prop_id[0] || (td[i].prop_id2 != ~0 && td[i].prop_id2 == prop_id[0]), + "expected property id %#x or %#x, got %#lx\n", + td[i].prop_id, td[i].prop_id2, prop_id[0]);
- if (status == Ok) - { - status = GdipGetPropertyItemSize(image, prop_id[0], &prop_size); - if (prop_count == 0) - expect(PropertyNotFound, status); - else - { - expect(Ok, status); + expect(Ok, status);
- assert(sizeof(item) >= prop_size); - ok(prop_size > sizeof(PropertyItem), "got too small prop_size %u\n", - prop_size); - ok(td[i].prop_size + sizeof(PropertyItem) == prop_size || - (td[i].prop_size2 != ~0 && td[i].prop_size2 + sizeof(PropertyItem) == prop_size), - "expected property size (%u or %u)+%u, got %u\n", - td[i].prop_size, td[i].prop_size2, (UINT) sizeof(PropertyItem), prop_size); - - status = GdipGetPropertyItem(image, prop_id[0], 0, &item.data); - ok(status == InvalidParameter || status == GenericError /* Win7 */, - "expected InvalidParameter, got %d\n", status); - status = GdipGetPropertyItem(image, prop_id[0], prop_size - 1, &item.data); - ok(status == InvalidParameter || status == GenericError /* Win7 */, - "expected InvalidParameter, got %d\n", status); - status = GdipGetPropertyItem(image, prop_id[0], prop_size + 1, &item.data); - ok(status == InvalidParameter || status == GenericError /* Win7 */, - "expected InvalidParameter, got %d\n", status); - status = GdipGetPropertyItem(image, prop_id[0], prop_size, &item.data); - expect(Ok, status); - ok(prop_id[0] == item.data.id, - "expected property id %#lx, got %#lx\n", prop_id[0], item.data.id); - } + assert(sizeof(item) >= prop_size); + ok(prop_size > sizeof(PropertyItem), "got too small prop_size %u\n", + prop_size); + ok(td[i].prop_size + sizeof(PropertyItem) == prop_size || + (td[i].prop_size2 != ~0 && td[i].prop_size2 + sizeof(PropertyItem) == prop_size), + "expected property size (%u or %u)+%u, got %u\n", + td[i].prop_size, td[i].prop_size2, (UINT) sizeof(PropertyItem), prop_size); + + status = GdipGetPropertyItem(image, prop_id[0], 0, &item.data); + ok(status == InvalidParameter || status == GenericError /* Win7 */, + "expected InvalidParameter, got %d\n", status); + status = GdipGetPropertyItem(image, prop_id[0], prop_size - 1, &item.data); + ok(status == InvalidParameter || status == GenericError /* Win7 */, + "expected InvalidParameter, got %d\n", status); + status = GdipGetPropertyItem(image, prop_id[0], prop_size + 1, &item.data); + ok(status == InvalidParameter || status == GenericError /* Win7 */, + "expected InvalidParameter, got %d\n", status); + status = GdipGetPropertyItem(image, prop_id[0], prop_size, &item.data); + expect(Ok, status); + ok(prop_id[0] == item.data.id, + "expected property id %#lx, got %#lx\n", prop_id[0], item.data.id); }
GdipDisposeImage(image);
From: Jeff Smith whydoubt@gmail.com
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54748 --- dlls/gdiplus/image.c | 7 ++++++- dlls/gdiplus/tests/image.c | 30 +++++++++++++----------------- 2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 3b9ce3c14f1..7b2aca7c9f7 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -2762,7 +2762,12 @@ GpStatus WINGDIPAPI GdipGetPropertySize(GpImage *image, UINT *size, UINT *count) }
reader = ((GpBitmap *)image)->metadata_reader; - if (!reader) return PropertyNotFound; + if (!reader) + { + *count = 0; + *size = 0; + return Ok; + }
hr = IWICMetadataReader_GetCount(reader, &prop_count); if (FAILED(hr)) return hresult_to_status(hr); diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 27b6bc9b2de..3d0d5d7a16f 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3661,7 +3661,7 @@ static void test_image_properties(void) { gifimage, sizeof(gifimage), ImageTypeBitmap, 1, 4, 4, ~0, 0x5100, ~0, 16 }, { wmfimage, sizeof(wmfimage), ImageTypeMetafile, 0, ~0, 0, ~0, 0, ~0, -GenericError } }; - GpStatus status; + GpStatus status, expected; GpImage *image; UINT prop_count, prop_size, i; PROPID prop_id[16] = { 0 }; @@ -3786,6 +3786,18 @@ static void test_image_properties(void) "expected property id %#lx, got %#lx\n", prop_id[0], item.data.id); }
+ status = GdipGetPropertySize(NULL, &prop_size, &prop_count); + expect(InvalidParameter, status); + status = GdipGetPropertySize(image, &prop_size, NULL); + expect(InvalidParameter, status); + status = GdipGetPropertySize(image, NULL, &prop_count); + expect(InvalidParameter, status); + status = GdipGetPropertySize(image, NULL, NULL); + expect(InvalidParameter, status); + expected = (image_type == ImageTypeMetafile) ? NotImplemented : Ok; + status = GdipGetPropertySize(image, &prop_size, &prop_count); + expect(expected, status); + GdipDisposeImage(image);
winetest_pop_context(); @@ -4150,14 +4162,6 @@ static void test_GdipGetAllPropertyItems(void)
HeapFree(GetProcessHeap(), 0, prop_id);
- status = GdipGetPropertySize(NULL, &total_size, &total_count); - expect(InvalidParameter, status); - status = GdipGetPropertySize(image, &total_size, NULL); - expect(InvalidParameter, status); - status = GdipGetPropertySize(image, NULL, &total_count); - expect(InvalidParameter, status); - status = GdipGetPropertySize(image, NULL, NULL); - expect(InvalidParameter, status); total_size = 0xdeadbeef; total_count = 0xdeadbeef; status = GdipGetPropertySize(image, &total_size, &total_count); @@ -4958,14 +4962,6 @@ static void test_gif_properties(void)
HeapFree(GetProcessHeap(), 0, prop_id);
- status = GdipGetPropertySize(NULL, &total_size, &total_count); - expect(InvalidParameter, status); - status = GdipGetPropertySize(image, &total_size, NULL); - expect(InvalidParameter, status); - status = GdipGetPropertySize(image, NULL, &total_count); - expect(InvalidParameter, status); - status = GdipGetPropertySize(image, NULL, NULL); - expect(InvalidParameter, status); total_size = 0xdeadbeef; total_count = 0xdeadbeef; status = GdipGetPropertySize(image, &total_size, &total_count);
From: Jeff Smith whydoubt@gmail.com
Discovered in researching https://bugs.winehq.org/show_bug.cgi?id=54748 --- dlls/gdiplus/image.c | 2 +- dlls/gdiplus/tests/image.c | 58 +++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 7b2aca7c9f7..ea851cf0be5 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -2848,7 +2848,7 @@ GpStatus WINGDIPAPI GdipGetAllPropertyItems(GpImage *image, UINT size, }
reader = ((GpBitmap *)image)->metadata_reader; - if (!reader) return PropertyNotFound; + if (!reader) return GenericError;
hr = IWICMetadataReader_GetEnumerator(reader, &enumerator); if (FAILED(hr)) return hresult_to_status(hr); diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 3d0d5d7a16f..a8104befd21 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3662,7 +3662,9 @@ static void test_image_properties(void) { wmfimage, sizeof(wmfimage), ImageTypeMetafile, 0, ~0, 0, ~0, 0, ~0, -GenericError } }; GpStatus status, expected; + BOOL broken_ok; GpImage *image; + PropertyItem *prop_item; UINT prop_count, prop_size, i; PROPID prop_id[16] = { 0 }; ImageType image_type; @@ -3798,6 +3800,36 @@ static void test_image_properties(void) status = GdipGetPropertySize(image, &prop_size, &prop_count); expect(expected, status);
+ prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size); + status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL); + expect(InvalidParameter, status); + status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL); + expect(InvalidParameter, status); + status = GdipGetAllPropertyItems(image, 0, 0, NULL); + expect(InvalidParameter, status); + expected = (image_type == ImageTypeMetafile) ? NotImplemented : InvalidParameter; + broken_ok = (td[i].prop_count == 1 || td[i].prop_count2 == 1); + status = GdipGetAllPropertyItems(image, prop_size, 1, prop_item); + ok(status == expected || broken(status == Ok && broken_ok), /* XP */ + "Expected %d, got %d\n", expected, status); + status = GdipGetAllPropertyItems(image, prop_size + 1, prop_count, prop_item); + expect(expected, status); + if (image_type != ImageTypeMetafile && prop_count == 0) + expected = GenericError; + status = GdipGetAllPropertyItems(image, 0, 0, prop_item); + broken_ok = (td[i].prop_count == 0 || td[i].prop_count2 == 0); + ok(status == expected || broken(status == Ok && broken_ok), /* XP */ + "Expected %d, got %d\n", expected, status); + status = GdipGetAllPropertyItems(image, 0, prop_count, prop_item); + ok(status == expected || broken(status == Ok && broken_ok), /* XP */ + "Expected %d, got %d\n", expected, status); + if (expected == InvalidParameter) + expected = Ok; + status = GdipGetAllPropertyItems(image, prop_size, prop_count, prop_item); + ok(status == expected || broken(status == Ok && broken_ok), /* XP */ + "Expected %d, got %d\n", expected, status); + HeapFree(GetProcessHeap(), 0, prop_item); + GdipDisposeImage(image);
winetest_pop_context(); @@ -4172,19 +4204,6 @@ static void test_GdipGetAllPropertyItems(void) "expected total property size %u, got %u\n", prop_size, total_size);
prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size); - - status = GdipGetAllPropertyItems(image, 0, prop_count, prop_item); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size, 1, prop_item); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, 0, 0, NULL); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size + 1, prop_count, prop_item); - expect(InvalidParameter, status); status = GdipGetAllPropertyItems(image, prop_size, prop_count, prop_item); expect(Ok, status);
@@ -4972,19 +4991,6 @@ static void test_gif_properties(void) "expected total property size %u, got %u\n", prop_size, total_size);
prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size); - - status = GdipGetAllPropertyItems(image, 0, prop_count, prop_item); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size, 1, prop_item); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, 0, 0, NULL); - expect(InvalidParameter, status); - status = GdipGetAllPropertyItems(image, prop_size + 1, prop_count, prop_item); - expect(InvalidParameter, status); status = GdipGetAllPropertyItems(image, prop_size, prop_count, prop_item); expect(Ok, status);
Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c:
status = GdipGetPropertySize(image, &prop_size, &prop_count); expect(expected, status);
prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size);status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL);expect(InvalidParameter, status);status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL);expect(InvalidParameter, status);
This looks like the same test repeated, is that intentional?
Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c:
status = GdipGetPropertySize(image, &prop_size, &prop_count); expect(expected, status);
prop_item = HeapAlloc(GetProcessHeap(), 0, prop_size);status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL);expect(InvalidParameter, status);status = GdipGetAllPropertyItems(image, prop_size, prop_count, NULL);expect(InvalidParameter, status);status = GdipGetAllPropertyItems(image, 0, 0, NULL);expect(InvalidParameter, status);expected = (image_type == ImageTypeMetafile) ? NotImplemented : InvalidParameter;broken_ok = (td[i].prop_count == 1 || td[i].prop_count2 == 1);status = GdipGetAllPropertyItems(image, prop_size, 1, prop_item);ok(status == expected || broken(status == Ok && broken_ok), /* XP */"Expected %d, got %d\n", expected, status);
I don't think this is really broken if `prop_count` happens to be 1?
On Sat Jul 15 19:50:10 2023 +0000, Esme Povirk wrote:
This looks like the same test repeated, is that intentional?
Where these tests were moved from also had this repeat. It does not appear to have any value. I will clean that up in the next iteration.
On Sat Jul 15 19:55:29 2023 +0000, Esme Povirk wrote:
I don't think this is really broken if `prop_count` happens to be 1?
Agreed. I'm re-working this set of tests.
-
Esme Povirk (＠madewokherd) -
Jeff Smith
-
Jeffrey Smith (＠whydoubt)