[PATCH 0/4] MR8330: comctl32/listview: Miscellaneous fixes for uninitialized and unnecessarily initialized variables.
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/comctl32/tests/listview.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index b767106e97a..e57c11bd3e6 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -1160,14 +1160,32 @@ static void test_images(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); + /* + * If LVS_EX_SUBITEMIMAGES is not set, iImage field value is untouched + * for subitems. + */ memset(&item, 0, sizeof(item)); item.mask = LVIF_IMAGE; item.iSubItem = 1; + item.iImage = 500; r = SendMessageA(hwnd, LVM_GETITEMA, 0, (LPARAM)&item); ok(r, "Failed to get item.\n"); + todo_wine ok(item.iImage == 500, "Unexpected iImage value %d.\n", item.iImage); ok_sequence(sequences, PARENT_SEQ_INDEX, empty_seq, "get image dispinfo 2", FALSE); + r = SendMessageA(hwnd, LVM_SETEXTENDEDLISTVIEWSTYLE, 0, LVS_EX_SUBITEMIMAGES); + ok(!r, "Unexpected return value %d.\n", r); + + memset(&item, 0, sizeof(item)); + item.mask = LVIF_IMAGE; + item.iSubItem = 1; + item.iImage = 500; + r = SendMessageA(hwnd, LVM_GETITEMA, 0, (LPARAM)&item); + ok(r, "Failed to get item.\n"); + ok(item.iImage == I_IMAGECALLBACK, "Unexpected iImage value %d.\n", item.iImage); + + flush_sequences(sequences, NUM_MSG_SEQUENCES); DestroyWindow(hwnd); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8330
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/comctl32/listview.c | 2 -- dlls/comctl32/tests/listview.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 31f8a933e56..57ad887364f 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -6867,8 +6867,6 @@ static BOOL LISTVIEW_GetItemT(const LISTVIEW_INFO *infoPtr, LPLVITEMW lpLVItem, { if (!lpLVItem->iSubItem || (infoPtr->dwLvExStyle & LVS_EX_SUBITEMIMAGES)) lpLVItem->iImage = pItemHdr->iImage; - else - lpLVItem->iImage = 0; } /* The pszText field */ diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index e57c11bd3e6..d0e86e5f8aa 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -1170,7 +1170,7 @@ static void test_images(void) item.iImage = 500; r = SendMessageA(hwnd, LVM_GETITEMA, 0, (LPARAM)&item); ok(r, "Failed to get item.\n"); - todo_wine ok(item.iImage == 500, "Unexpected iImage value %d.\n", item.iImage); + ok(item.iImage == 500, "Unexpected iImage value %d.\n", item.iImage); ok_sequence(sequences, PARENT_SEQ_INDEX, empty_seq, "get image dispinfo 2", FALSE); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8330
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/comctl32/tests/listview.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index d0e86e5f8aa..4303fd2a8c5 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3498,6 +3498,8 @@ static void test_ownerdata(void) expect(1, res); res = SendMessageA(hwnd, LVM_GETITEMCOUNT, 0, 0); expect(1, res); + res = SendMessageA(hwnd, LVM_GETITEMSTATE, 0, 0xff); + todo_wine expect(2, res); DestroyWindow(hwnd); /* LVM_SETITEM and LVM_SETITEMTEXT is unsupported on LVS_OWNERDATA */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8330
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/comctl32/listview.c | 1 + dlls/comctl32/tests/listview.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 57ad887364f..43e2258d8c3 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -7292,6 +7292,7 @@ static UINT LISTVIEW_GetItemState(const LISTVIEW_INFO *infoPtr, INT nItem, UINT lvItem.iItem = nItem; lvItem.iSubItem = 0; lvItem.mask = LVIF_STATE; + lvItem.state = 0; lvItem.stateMask = uMask; if (!LISTVIEW_GetItemW(infoPtr, &lvItem)) return 0; diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index 4303fd2a8c5..3ca3149e22b 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -3499,7 +3499,7 @@ static void test_ownerdata(void) res = SendMessageA(hwnd, LVM_GETITEMCOUNT, 0, 0); expect(1, res); res = SendMessageA(hwnd, LVM_GETITEMSTATE, 0, 0xff); - todo_wine expect(2, res); + expect(2, res); DestroyWindow(hwnd); /* LVM_SETITEM and LVM_SETITEMTEXT is unsupported on LVS_OWNERDATA */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8330
Nikolay Sivov (@nsivov) commented about dlls/comctl32/tests/listview.c:
expect(1, res); res = SendMessageA(hwnd, LVM_GETITEMCOUNT, 0, 0); expect(1, res); + res = SendMessageA(hwnd, LVM_GETITEMSTATE, 0, 0xff); + todo_wine expect(2, res); DestroyWindow(hwnd);
If this is specific to LVS_OWNERDATA path, we need more tests with callback mask. If it's not specific to LVS_OWNERDATA, the test should be placed elsewhere. Note that return value is not an index, so please compare with defined state constants, and without expect(), just plain ok(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8330#note_106926
Nikolay Sivov (@nsivov) commented about dlls/comctl32/listview.c:
lvItem.iItem = nItem; lvItem.iSubItem = 0; lvItem.mask = LVIF_STATE; + lvItem.state = 0; lvItem.stateMask = uMask; if (!LISTVIEW_GetItemW(infoPtr, &lvItem)) return 0;
We need LVM_GETITEM test too for this, the fix looks misplaced. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8330#note_106927
Nikolay Sivov (@nsivov) commented about dlls/comctl32/tests/listview.c:
ok(r, "Failed to get item.\n"); + todo_wine ok(item.iImage == 500, "Unexpected iImage value %d.\n", item.iImage);
ok_sequence(sequences, PARENT_SEQ_INDEX, empty_seq, "get image dispinfo 2", FALSE);
+ r = SendMessageA(hwnd, LVM_SETEXTENDEDLISTVIEWSTYLE, 0, LVS_EX_SUBITEMIMAGES); + ok(!r, "Unexpected return value %d.\n", r); + + memset(&item, 0, sizeof(item)); + item.mask = LVIF_IMAGE; + item.iSubItem = 1; + item.iImage = 500; + r = SendMessageA(hwnd, LVM_GETITEMA, 0, (LPARAM)&item); + ok(r, "Failed to get item.\n"); + ok(item.iImage == I_IMAGECALLBACK, "Unexpected iImage value %d.\n", item.iImage); +
The test looks fine, but I think using some value that's not I_IMAGECALLBACK would be better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8330#note_106928
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Nikolay Sivov (＠nsivov)