[PATCH v2 05/10] comctl32/listbox: Use a helper to retrieve item data by index

[Huw Davies]

On Tue, Feb 12, 2019 at 12:51:28PM +0200, Gabriel Ivăncescu wrote:
> On 2/12/19 10:59 AM, Huw Davies wrote:
> > On Mon, Feb 11, 2019 at 07:03:08PM +0200, Gabriel Ivăncescu wrote:
> > > Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> > > ---
> > >   dlls/comctl32/listbox.c | 7 ++++++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> > > index c84d317..ec6a169 100644
> > > --- a/dlls/comctl32/listbox.c
> > > +++ b/dlls/comctl32/listbox.c
> > > @@ -160,6 +160,11 @@ static BOOL is_item_selected( const LB_DESCR *descr, UINT index )
> > >       return descr->items[index].selected;
> > >   }
> > > +static ULONG_PTR get_item_data( const LB_DESCR *descr, UINT index )
> > > +{
> > > +    return (descr->style & LBS_NODATA) ? 0 : descr->items[index].data;
> > > +}
> > > +
> > >   /***********************************************************************
> > >    *           LISTBOX_GetCurrentPageSize
> > >    *
> > > @@ -565,7 +570,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
> > >           if (focused)
> > >               dis.itemState |= ODS_FOCUS;
> > >           if (!IsWindowEnabled(descr->self)) dis.itemState |= ODS_DISABLED;
> > > -        dis.itemData     = item ? item->data : 0;
> > > +        dis.itemData     = get_item_data(descr, index);
> > >           dis.rcItem       = *rect;
> > >           TRACE("[%p]: drawitem %d (%s) action=%02x state=%02x rect=%s\n",
> > >                 descr->self, index, item ? debugstr_w(item->str) : "", action,
> > 
> > This helper should now be used throughout the code, for example
> > LISTBOX_GetText(), _FindStringPos(), _DeleteItem(), handler for
> > LB_GETITEMDATA and ideally _FindString() (but I'll let you off
> > _FindString() since that will require a bit of rewriting).
> > 
> > You'll see that doing this will further reduce the _NODATA special
> > cases.
> > 
> > The same then goes for the following patch which introduces
> > get_item_string().
> > 
> > Huw.
> > 
> 
> I think FindStringPos also fails with NODATA, should I still use it there or
> leave it as it is to keep the patch smaller?

Ideally every read of ->data/->str would be replaced by the appropriate helper.
I realise that _FineStringPos will require a little bit of work, but, since you ask,
I think it's worth it.

This should also answer your question about DeleteItem.

Huw.