Only increase the item array if we actually have to. Previously, sending for example just 1 to nb_items repeatedly would always increase the array by LB_ARRAY_GRANULARITY, even if there was plenty of space.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Assume LB_ARRAY_GRANULARITY is a power of 2 and note it in the comments. This simplifies the code (and it's already the case anyway).
dlls/comctl32/listbox.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef8..5c171ab 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -41,7 +41,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(listbox);
-/* Items array granularity */ +/* Items array granularity; must be a power of 2 */ #define LB_ARRAY_GRANULARITY 16
/* Scrolling timeout in ms */ @@ -673,16 +673,18 @@ 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 += HeapSize( GetProcessHeap(), 0, descr->items ) / sizeof(*item); - item = HeapReAlloc( GetProcessHeap(), 0, descr->items, - nb_items * sizeof(LB_ITEMDATA)); + nb_items += descr->nb_items; + if (nb_items > HeapSize(GetProcessHeap(), 0, descr->items) / sizeof(*item)) + { + UINT n = (nb_items + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1); + item = HeapReAlloc(GetProcessHeap(), 0, descr->items, n * sizeof(*item)); + } + else return LB_OKAY; } else { - item = HeapAlloc( GetProcessHeap(), 0, - nb_items * sizeof(LB_ITEMDATA)); + UINT n = (nb_items + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1); + item = HeapAlloc(GetProcessHeap(), 0, n * sizeof(*item)); }
if (!item)
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/user32/listbox.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index c8bd148..af325d2 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -36,7 +36,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(listbox);
-/* Items array granularity */ +/* Items array granularity; must be a power of 2 */ #define LB_ARRAY_GRANULARITY 16
/* Scrolling timeout in ms */ @@ -698,16 +698,18 @@ 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 += HeapSize( GetProcessHeap(), 0, descr->items ) / sizeof(*item); - item = HeapReAlloc( GetProcessHeap(), 0, descr->items, - nb_items * sizeof(LB_ITEMDATA)); + nb_items += descr->nb_items; + if (nb_items > HeapSize(GetProcessHeap(), 0, descr->items) / sizeof(*item)) + { + UINT n = (nb_items + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1); + item = HeapReAlloc(GetProcessHeap(), 0, descr->items, n * sizeof(*item)); + } + else return LB_OKAY; } else { - item = HeapAlloc( GetProcessHeap(), 0, - nb_items * sizeof(LB_ITEMDATA)); + UINT n = (nb_items + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1); + item = HeapAlloc(GetProcessHeap(), 0, n * sizeof(*item)); }
if (!item)
I think it's better to introduce 'capacity' or 'nb_allocated' field, and use that instead of HeapSize().
Then you can have new array length aligned in one place, with heap_realloc() that will handle initial state of items == NULL.
On Mon, Nov 5, 2018 at 2:14 PM Nikolay Sivov nsivov@codeweavers.com wrote:
I think it's better to introduce 'capacity' or 'nb_allocated' field, and use that instead of HeapSize().
Then you can have new array length aligned in one place, with heap_realloc() that will handle initial state of items == NULL.
I feel as if it's out of the scope of this (small) patch. That would require quite some changes since a lot of the other code also uses HeapSize().
Also, I'm probably misunderstanding something, but HeapSize is aligned already, though. It's just the current code which rounds up the size requested *without* looking at the available space. So it will always grow, even if there's plenty of free space.
-
Gabriel Ivăncescu -
Nikolay Sivov