[PATCH 0/4] MR8330: comctl32/listview: Miscellaneous fixes for uninitialized and unnecessarily initialized variables.

[PATCH 1/4] comctl32/tests: Add tests for iImage value for listview subitems.

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 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);
 }

[PATCH 2/4] comctl32/listview: Don't touch iImage value for subitems if LVS_EX_SUBITEMIMAGES is not set.

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 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);

[PATCH 3/4] comctl32/tests: Add a test for LVM_GETITEMSTATE.

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 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 */

[PATCH 4/4] comctl32/listview: Initialize state value to 0 in LISTVIEW_GetItemState().

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 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 */

[Nikolay Sivov (＠nsivov)]

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().

[Nikolay Sivov (＠nsivov)]

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.

[Nikolay Sivov (＠nsivov)]

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.