[PATCH v2 2/4] comctl32/listbox: Rewrite FindString to use helpers and avoid the macro

[Huw Davies]

On Tue, Feb 26, 2019 at 02:53:36PM +0200, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
> 
> no-op patch.
> 
> This has been cleaned up so now all the comparison code (for each of the
> 3 variants) is only written once because the outer loop will wrap around,
> which also avoids the ugly macro. Of course it also uses the helpers and
> avoids having a pointless item variable (especially since we already tracked
> the index to begin with).
> 
>  dlls/comctl32/listbox.c | 66 ++++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 30 deletions(-)
> 
> diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> index 954866a..dd29dc6 100644
> --- a/dlls/comctl32/listbox.c
> +++ b/dlls/comctl32/listbox.c
> @@ -965,46 +965,48 @@ static INT LISTBOX_FindFileStrPos( LB_DESCR *descr, LPCWSTR str )
>   */
>  static INT LISTBOX_FindString( LB_DESCR *descr, INT start, LPCWSTR str, BOOL exact )
>  {
> -    INT i;
> -    LB_ITEMDATA *item;
> +    INT i, end = descr->nb_items;
>  
>      if (descr->style & LBS_NODATA) return LB_ERR;
>  
> -    if (start >= descr->nb_items) start = -1;
> -    item = descr->items + start + 1;
> +    start++;
> +    if (start >= descr->nb_items) start = 0;
>      if (HAS_STRINGS(descr))
>      {
>          if (!str || ! str[0] ) return LB_ERR;
>          if (exact)
>          {
> -            for (i = start + 1; i < descr->nb_items; i++, item++)
> -                if (!LISTBOX_lstrcmpiW( descr->locale, str, item->str )) return i;
> -            for (i = 0, item = descr->items; i <= start; i++, item++)
> -                if (!LISTBOX_lstrcmpiW( descr->locale, str, item->str )) return i;
> +            for (;;)
> +            {
> +                for (i = start; i < end; i++)
> +                    if (!LISTBOX_lstrcmpiW(descr->locale, str, get_item_string(descr, i)))
> +                        return i;
> +                if (!start) break;
> +                end = start;
> +                start = 0;
> +            }
>          }


These nested for loops are ugly.  What about something like this (having set up
start as you've already done):

    for (i = 0, item = start; i < descr->nb_items; i++, item++)
    {
        if (item == descr->nb_items) item = 0;
        if (!LISTBOX_lstrcmpiW(descr->locale, str, get_item_string(descr, item)))
            return item;
    }

Huw.