[PATCH resend 1/2] comctl32/listbox: Fix InitStorage heap extension
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(a)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) -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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) -- 2.19.1
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(a)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.
participants (2)
-
Gabriel Ivăncescu -
Nikolay Sivov