Re: [PATCH v3 3/6] shell32/autocomplete: Implement a cache and sort the enumerated strings for proper behavior

[Huw Davies]

On Thu, Oct 25, 2018 at 09:04:52PM +0300, Gabriel Ivăncescu wrote:
> Windows doesn't reset and re-enumerate it everytime autocompletion happens,
> and it also sorts the strings. This matches it more closely and makes it
> more useable on large lists as well.
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/shell32/autocomplete.c | 207 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 157 insertions(+), 50 deletions(-)
>
> diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c
> index b3f86f3..9cfce4b 100644
> --- a/dlls/shell32/autocomplete.c
> +++ b/dlls/shell32/autocomplete.c
> @@ -25,7 +25,6 @@
> -
>
> implement ACO_FILTERPREFIXES style
> -
>
> implement ACO_RTLREADING style
> -
>
> implement ResetEnumerator
> -
> - string compares should be case-insensitive, the content of the list should be sorted
>
> */
>
> #include "config.h"
> @@ -62,6 +61,8 @@ typedef struct
>      LONG ref;
>      BOOL initialized;
>      BOOL enabled;
> - UINT enum_strs_num;
> - WCHAR **enum_strs;
> HWND hwndEdit;
> HWND hwndListBox;
> WNDPROC wpOrigEditProc;
>
> @@ -103,10 +104,102 @@ static void set_text_and_selection(IAutoCompleteImpl *ac, HWND hwnd, WCHAR *text
>          CallWindowProcW(proc, hwnd, EM_SETSEL, start, end);
>  }
>
> +static int enumerate_strings_cmpfn(const void *a, const void *b)
> +{
> - return strcmpiW(*(WCHAR* const*)a, *(WCHAR* const*)b);
>
> +}
> -
>
> +static void enumerate_strings(IAutoCompleteImpl *ac)
> +{
> - /*
> -
>   Enumerate all of the strings and sort them in the internal list.
> -
> -
>   We don't free the enumerated strings (except on error) to avoid needless
> -
>   copies, until the next reset (or the object itself is destroyed)
> - */
> - UINT i, cur, array_size = 1024, curblock_size = array_size, numstrs = 0;
> - LPOLESTR *strs = NULL, *tmp;
> -
> - for (;;)
> - {
> -
>    LONG rem;
> -
>    BOOL break_enum = FALSE;
> -
> -
>    if ((tmp = heap_realloc(strs, array_size * sizeof(*strs))) == NULL)
> -
>        goto fail;
> -
>    strs = tmp;
> -
>    rem = curblock_size;
> -
> -
>    while (rem > 0)
> -
>    {
> -
>        ULONG n = 0;
> -
>        cur = array_size - rem;
> -
>        IEnumString_Next(ac->enumstr, rem, &strs[cur], &n);
> -
>        if (n == 0)
> -
>        {
> -
>            break_enum = TRUE;
> -
>            break;
> -
>        }
> -
>        rem -= n;
> -
>    }
> -
>    if (break_enum) break;
> -
>    curblock_size = array_size;
> -
>    array_size += curblock_size;
> - }
> -
> - /* Allocate even if there were zero strings enumerated, to mark it non-NULL */
> - numstrs = cur;

There are too many variables tracking size in this block which makes the whole
thing confusing.  You should just need array_size, cur and n.   If you really
need a boolean to break out of the outer loop then name that variable 'done'
and change the for (;;) -> while (!done)

Huw.