[PATCH 1/5] user32/listbox: Check for out of bounds using the index when painting the item

[Gabriel Ivăncescu]

Needed for LBS_NODATA.

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---

user32 versions of the comctl32 patchset.

 dlls/user32/listbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 594c956..821da60 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -533,7 +533,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
         RECT r;
         HRGN hrgn;
 
-	if (!item)
+	if (index >= descr->nb_items)
 	{
 	    if (action == ODA_FOCUS)
 		DrawFocusRect( hdc, rect );
-- 
2.19.1

[PATCH 2/5] user32/listbox: Use a helper to expand the item array

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 64 +++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 39 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 821da60..80ba38b 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -33,10 +33,11 @@
 #include "controls.h"
 #include "wine/exception.h"
 #include "wine/debug.h"
+#include "wine/heap.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(listbox);
 
-/* Items array granularity */
+/* Items array granularity (must be power of 2) */
 #define LB_ARRAY_GRANULARITY 16
 
 /* Scrolling timeout in ms */
@@ -121,6 +122,26 @@ static TIMER_DIRECTION LISTBOX_Timer = LB_TIMER_NONE;
 
 static LRESULT LISTBOX_GetItemRect( const LB_DESCR *descr, INT index, RECT *rect );
 
+static BOOL resize_storage(LB_DESCR *descr, UINT items_size)
+{
+    LB_ITEMDATA *items;
+
+    if (items_size > descr->items_size ||
+        items_size + LB_ARRAY_GRANULARITY * 2 < descr->items_size)
+    {
+        items_size = (items_size + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1);
+        items = heap_realloc(descr->items, items_size * sizeof(LB_ITEMDATA));
+        if (!items)
+        {
+            SEND_NOTIFICATION(descr, LBN_ERRSPACE);
+            return FALSE;
+        }
+        descr->items_size = items_size;
+        descr->items = items;
+    }
+    return TRUE;
+}
+
 static BOOL is_item_selected( const LB_DESCR *descr, UINT index )
 {
     return descr->items[index].selected;
@@ -709,27 +730,10 @@ static void LISTBOX_DrawFocusRect( LB_DESCR *descr, BOOL on )
  */
 static LRESULT LISTBOX_InitStorage( LB_DESCR *descr, INT nb_items )
 {
-    LB_ITEMDATA *item;
-
-    nb_items += LB_ARRAY_GRANULARITY - 1;
-    nb_items -= (nb_items % LB_ARRAY_GRANULARITY);
-    if (descr->items) {
-        nb_items += descr->items_size;
-	item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
-                              nb_items * sizeof(LB_ITEMDATA));
-    }
-    else {
-	item = HeapAlloc( GetProcessHeap(), 0,
-                              nb_items * sizeof(LB_ITEMDATA));
-    }
+    UINT new_size = descr->nb_items + nb_items;
 
-    if (!item)
-    {
-        SEND_NOTIFICATION( descr, LBN_ERRSPACE );
+    if (new_size > descr->items_size && !resize_storage(descr, new_size))
         return LB_ERRSPACE;
-    }
-    descr->items_size = nb_items;
-    descr->items = item;
     return LB_OKAY;
 }
 
@@ -1534,29 +1538,11 @@ static LRESULT LISTBOX_InsertItem( LB_DESCR *descr, INT index,
                                    LPWSTR str, ULONG_PTR data )
 {
     LB_ITEMDATA *item;
-    INT max_items;
     INT oldfocus = descr->focus_item;
 
     if (index == -1) index = descr->nb_items;
     else if ((index < 0) || (index > descr->nb_items)) return LB_ERR;
-    if (descr->nb_items == descr->items_size)
-    {
-        /* We need to grow the array */
-        max_items = descr->items_size + LB_ARRAY_GRANULARITY;
-	if (descr->items)
-    	    item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
-                                  max_items * sizeof(LB_ITEMDATA) );
-	else
-	    item = HeapAlloc( GetProcessHeap(), 0,
-                                  max_items * sizeof(LB_ITEMDATA) );
-        if (!item)
-        {
-            SEND_NOTIFICATION( descr, LBN_ERRSPACE );
-            return LB_ERRSPACE;
-        }
-        descr->items_size = max_items;
-        descr->items = item;
-    }
+    if (!resize_storage(descr, descr->nb_items + 1)) return LB_ERR;
 
     /* Insert the item structure */
 
-- 
2.19.1

[PATCH 2/5] user32/listbox: Use a helper to expand the item array

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=47329

Your paranoid android.


=== debian9 (32 bit WoW report) ===

user32:
menu.c:2354: Test failed: test 21
menu: Timeout

[PATCH 2/5] user32/listbox: Use a helper to expand the item array

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 3/5] user32/listbox: Shrink the item array with resize_storage

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 80ba38b..3bd1b58 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -1678,7 +1678,6 @@ static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index )
 static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index )
 {
     LB_ITEMDATA *item;
-    INT max_items;
 
     if ((index < 0) || (index >= descr->nb_items)) return LB_ERR;
 
@@ -1698,20 +1697,8 @@ static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index )
                        (descr->nb_items - index) * sizeof(LB_ITEMDATA) );
     if (descr->anchor_item == descr->nb_items) descr->anchor_item--;
 
-    /* Shrink the item array if possible */
+    resize_storage(descr, descr->nb_items);
 
-    max_items = descr->items_size;
-    if (descr->nb_items < max_items - 2*LB_ARRAY_GRANULARITY)
-    {
-        max_items -= LB_ARRAY_GRANULARITY;
-        item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
-                            max_items * sizeof(LB_ITEMDATA) );
-        if (item)
-        {
-            descr->items_size = max_items;
-            descr->items = item;
-        }
-    }
     /* Repaint the items */
 
     LISTBOX_UpdateScroll( descr );
-- 
2.19.1

[PATCH 3/5] user32/listbox: Shrink the item array with resize_storage

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=47330

Your paranoid android.


=== debian9 (32 bit report) ===

user32:
msg.c:8713: Test failed: WaitForSingleObject failed 102
msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead
msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead
msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000

=== debian9 (32 bit WoW report) ===

user32:
menu.c:2354: Test failed: test 21
menu: Timeout

[PATCH 3/5] user32/listbox: Shrink the item array with resize_storage

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 4/5] user32/listbox: Return proper value for LB_INITSTORAGE

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 3bd1b58..a2791eb 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -734,7 +734,7 @@ static LRESULT LISTBOX_InitStorage( LB_DESCR *descr, INT nb_items )
 
     if (new_size > descr->items_size && !resize_storage(descr, new_size))
         return LB_ERRSPACE;
-    return LB_OKAY;
+    return descr->items_size;
 }
 
 
-- 
2.19.1

[PATCH 4/5] user32/listbox: Return proper value for LB_INITSTORAGE

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=47331

Your paranoid android.


=== debian9 (32 bit Chinese:China report) ===

user32:
input.c:2064: Test failed: expected WM_NCHITTEST message

=== debian9 (64 bit WoW report) ===

user32:
msg.c:8713: Test failed: WaitForSingleObject failed 102
msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead
msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead
msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000

[PATCH 4/5] user32/listbox: Return proper value for LB_INITSTORAGE

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 5/5] user32/tests: Add tests for LB_INITSTORAGE

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/tests/listbox.c | 64 +++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/dlls/user32/tests/listbox.c b/dlls/user32/tests/listbox.c
index b87acd7..7f1e9cf 100644
--- a/dlls/user32/tests/listbox.c
+++ b/dlls/user32/tests/listbox.c
@@ -1824,6 +1824,69 @@ todo_wine
     DestroyWindow(parent);
 }
 
+static void test_init_storage( void )
+{
+    static const DWORD styles[] =
+    {
+        LBS_HASSTRINGS,
+        LBS_NODATA | LBS_OWNERDRAWFIXED,
+    };
+    HWND parent, listbox;
+    LONG ret, items_size;
+    int i, j;
+
+    parent = create_parent();
+    for (i = 0; i < ARRAY_SIZE(styles); i++)
+    {
+        listbox = CreateWindowA("listbox", "TestList", styles[i] | WS_CHILD,
+                                0, 0, 100, 100, parent, (HMENU)1, NULL, 0);
+
+        items_size = SendMessageA(listbox, LB_INITSTORAGE, 100, 0);
+        ok(items_size >= 100, "expected at least 100, got %d\n", items_size);
+
+        ret = SendMessageA(listbox, LB_INITSTORAGE, 0, 0);
+        ok(ret == items_size, "expected %d, got %d\n", items_size, ret);
+
+        /* it doesn't grow since the space was already reserved */
+        ret = SendMessageA(listbox, LB_INITSTORAGE, items_size, 0);
+        ok(ret == items_size, "expected %d, got %d\n", items_size, ret);
+
+        /* it doesn't shrink the reserved space */
+        ret = SendMessageA(listbox, LB_INITSTORAGE, 42, 0);
+        ok(ret == items_size, "expected %d, got %d\n", items_size, ret);
+
+        /* now populate almost all of it so it's not reserved anymore */
+        if (styles[i] & LBS_NODATA)
+        {
+            ret = SendMessageA(listbox, LB_SETCOUNT, items_size - 1, 0);
+            ok(ret == 0, "unexpected return value %d\n", ret);
+        }
+        else
+        {
+            for (j = 0; j < items_size - 1; j++)
+            {
+                ret = SendMessageA(listbox, LB_INSERTSTRING, -1, (LPARAM)"");
+                ok(ret == j, "expected %d, got %d\n", j, ret);
+            }
+        }
+
+        /* we still have one more reserved slot, so it doesn't grow yet */
+        ret = SendMessageA(listbox, LB_INITSTORAGE, 1, 0);
+        ok(ret == items_size, "expected %d, got %d\n", items_size, ret);
+
+        /* fill the slot and check again, it should grow this time */
+        ret = SendMessageA(listbox, LB_INSERTSTRING, -1, (LPARAM)"");
+        ok(ret == items_size - 1, "expected %d, got %d\n", items_size - 1, ret);
+        ret = SendMessageA(listbox, LB_INITSTORAGE, 0, 0);
+        ok(ret == items_size, "expected %d, got %d\n", items_size, ret);
+        ret = SendMessageA(listbox, LB_INITSTORAGE, 1, 0);
+        ok(ret > items_size, "expected it to grow past %d, got %d\n", items_size, ret);
+
+        DestroyWindow(listbox);
+    }
+    DestroyWindow(parent);
+}
+
 static void test_missing_lbuttonup( void )
 {
     HWND listbox, parent, capture;
@@ -2251,6 +2314,7 @@ START_TEST(listbox)
   test_listbox_LB_DIR();
   test_listbox_dlgdir();
   test_set_count();
+  test_init_storage();
   test_GetListBoxInfo();
   test_missing_lbuttonup();
   test_extents();
-- 
2.19.1

[PATCH 5/5] user32/tests: Add tests for LB_INITSTORAGE

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=47328

Your paranoid android.


=== debian9 (32 bit WoW report) ===

user32:
menu.c:2354: Test failed: test 21
menu: Timeout

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>