[PATCH v2 08/10] comctl32/listbox: Send LB_RESETCONTENT from RemoveItem rather than DeleteItem

[Huw Davies]

On Mon, Feb 11, 2019 at 07:03:11PM +0200, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
> 
> Ok, this is how I ended up making the code simpler later. The reason being
> that it really makes no sense at all to send LB_RESETCONTENT from within
> DeleteItem: that function is called *from* ResetContent itself and it only
> happened to work since it updated nb_items afterward. It was too fragile
> to begin with.
> 
> Note that LB_RESETCONTENT already calls DeleteItem, so we can simply re-use
> that, and IMO this makes much more sense and will simplify the code in the
> last patch.

Good, this is clearly how it should be done.

 
>  dlls/comctl32/listbox.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> index 633e7c6..0f74e80 100644
> --- a/dlls/comctl32/listbox.c
> +++ b/dlls/comctl32/listbox.c
> @@ -1660,13 +1660,9 @@ static LRESULT LISTBOX_InsertString( LB_DESCR *descr, INT index, LPCWSTR str )
>   */
>  static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index )
>  {
> -    /* save the item data before it gets freed by LB_RESETCONTENT */
>      ULONG_PTR item_data = descr->items[index].data;
>      LPWSTR item_str = descr->items[index].str;

As a nit-pick, there's no need to have these temp variables anymore,
you can just call the getter helpers as needed. (Yes, we'll call
get_item_data() twice, but it'll almost certainly get inlined anyway).

Huw.