Re: [PATCH] comctl32/listview: Fix the listview sorting error.

[Haoyang Chen]

hi,

lParam may hold the initial sequence. During the sorting process, this 
sequence is disrupted. But the initial queue order may still be used 
during sorting.
For example, the original queue: 6 4 5.
First sort result: 4 6 5.
Second sort result: 4 6 5.
Here it is wrong, because it should compare "6" and "5", but the index 
stored in the lParam of "6" is 0 (it has not changed), which is the 
first data, so it actually compares "4" and "5".

I will send V2 for the test error.

Thanks.

在 2021/3/8 上午11:22, Haoyang Chen 写道:
> The original sequence may be stored in lparam. During the sorting process,
> this sequence is out-of-order, but lparam remains unchanged.
> Signed-off-by: Haoyang Chen
>
> dlls/comctl32/dpa.c            | 13 ++++++--
>   dlls/comctl32/tests/listview.c | 59 ++++++++++++++++++++++++++++++++++
>   2 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/comctl32/dpa.c b/dlls/comctl32/dpa.c
> index 36b3a422e8c..69e9d0a33bb 100644
> --- a/dlls/comctl32/dpa.c
> +++ b/dlls/comctl32/dpa.c
> @@ -818,8 +818,17 @@ BOOL WINAPI DPA_Sort (HDPA hdpa, PFNDPACOMPARE pfnCompare, LPARAM lParam)
>       TRACE("(%p %p 0x%lx)\n", hdpa, pfnCompare, lParam);
>   if ((hdpa->nItemCount > 1) && (hdpa->ptrs))
> -
>    DPA_QuickSort (hdpa->ptrs, 0, hdpa->nItemCount - 1,
> -
>                   pfnCompare, lParam);
> -
>
> {
> -
>    LPVOID *ptrs = HeapAlloc (hdpa->hHeap, HEAP_ZERO_MEMORY,
> -
>                                         hdpa->nItemCount * sizeof(LPVOID));
> -
>    if (!ptrs)
> -
>            return FALSE;
> -
>    memcpy(ptrs, hdpa->ptrs, hdpa->nItemCount * sizeof(LPVOID));
> -
>    DPA_QuickSort (ptrs, 0, hdpa->nItemCount - 1, pfnCompare, lParam);
> -
> -
>    memcpy(hdpa->ptrs, ptrs, hdpa->nItemCount * sizeof(LPVOID));
> -
>    HeapFree (hdpa->hHeap, 0, ptrs);
> -
>
> }
>
> return TRUE;
> }
>
> diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c
> index e95b81f5bb1..7b036a9f6c1 100644
> --- a/dlls/comctl32/tests/listview.c
> +++ b/dlls/comctl32/tests/listview.c
> @@ -2921,6 +2921,28 @@ static INT WINAPI test_CallBackCompare(LPARAM first, LPARAM second, LPARAM lPara
>       return (first > second ? 1 : -1);
>   }
>
> +static INT WINAPI test_CallBackCompare1(LPARAM first, LPARAM second, LPARAM lParam)
> +{
> - CHAR str1[5];
> - CHAR str2[5];
> - INT  r;
> - HWND hwnd = (HWND)lParam;
> - LV_ITEMA item = {0};
> -
> - if (first == second) return 0;
> - item.cchTextMax = 5;
> - item.iSubItem = 0;
> - item.pszText = str1;
> - r = SendMessageA(hwnd, LVM_GETITEMTEXTA, first, (LPARAM)&item);
> - expect(TRUE, r);
> -
> - item.pszText = str2;
> - r = SendMessageA(hwnd, LVM_GETITEMTEXTA, second, (LPARAM)&item);
> - expect(TRUE, r);
> -
> - return atoi(str1) > atoi(str2) ? 1 : -1;
>
> +}
> - static void test_sorting(void)
> {
>     HWND hwnd;
>
> @@ -2929,6 +2951,9 @@ static void test_sorting(void)
>       LONG_PTR style;
>       static CHAR names[][5] = {"A", "B", "C", "D", "0"};
>       CHAR buff[10];
> -
>
> static CHAR before_sort_array[][5] = {"6","3","1","4","2"};
> -
>
> static CHAR after_sort_arary[][5] = {"1","2","3","4","6"};
> -
>
> INT i;
>
> hwnd = create_listview_control(LVS_REPORT);
>  ok(hwnd != NULL, "failed to create a listview window\n");
>
> @@ -2979,6 +3004,40 @@ static void test_sorting(void)
>   DestroyWindow(hwnd);
>
> - hwnd = create_listview_control(LVS_REPORT);
> - ok(hwnd != NULL, "failed to create a listview window\n");
> -
> - item.mask = LVIF_PARAM | LVIF_TEXT;
> - item.iSubItem = 0;
> - item.cchTextMax = 5;
> -
> - for (i = 0; i < sizeof(before_sort_array)/5; i++)
> - {
> -
>    item.iItem = i;
> -
>    item.lParam = i;
> -
>    item.pszText = &before_sort_array[i][0];
> -
>    r = SendMessageA(hwnd, LVM_INSERTITEMA, 0, (LPARAM)&item);
> -
>    expect(i, r);
> - }
> -
> - r = SendMessageA(hwnd, LVM_SORTITEMS, (WPARAM)(LPARAM)hwnd, (LPARAM)test_CallBackCompare1);
> - expect(TRUE, r);
> -
> - for (i = 0; i < sizeof(after_sort_arary)/5; i++)
> - {
> -
>    CHAR str[5];
> -
>    item.iItem = i;
> -
>    item.cchTextMax = 5;
> -
>    item.iSubItem = 0;
> -
>    item.pszText = str;
> -
>    r = SendMessageA(hwnd, LVM_GETITEMTEXTA, i, (LPARAM)&item);
> -
>    expect(TRUE, r);
> -
> -
>    expect(0, strcmp(str,&after_sort_arary[i][0]));
> - }
> -
> - DestroyWindow(hwnd);
> -
> /* switch to LVS_SORTASCENDING when some items added */
> hwnd = create_listview_control(LVS_REPORT);
> ok(hwnd != NULL, "failed to create a listview window\n");