[REGEDIT] Fix ListView selection and finding (#5150)

We will check the data size correctly, instead of 3 NUL byte appending hack. Add bSelectNone parameter to UpdateAddress and RefreshListView functions. If bSelectNone is TRUE, then select nothing of ListView. Fix item selection of ListView. Rename CompareData helper function as MatchData and improve it. Improve the search algorithm. If the item selection of ListView changed, scroll down to the item. Follow up to #5146. CORE-15986, CORE-18230

base/applications/regedit/childwnd.c

@@ -229,7 +229,7 @@ LRESULT CALLBACK AddressBarProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lPar
 }
 
 VOID
-UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath)
+UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone)
 {
     LPCWSTR keyPath, rootName;
     LPWSTR fullPath;
@@ -251,7 +251,7 @@ UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath)
 
     if (keyPath)
     {
-        RefreshListView(g_pChildWnd->hListWnd, hRootKey, keyPath);
+        RefreshListView(g_pChildWnd->hListWnd, hRootKey, keyPath, bSelectNone);
         rootName = get_root_key_name(hRootKey);
         cbFullPath = (wcslen(rootName) + 1 + wcslen(keyPath) + 1) * sizeof(WCHAR);
         fullPath = malloc(cbFullPath);

base/applications/regedit/find.c

@@ -87,41 +87,32 @@ static BOOL CompareName(LPCWSTR pszName1, LPCWSTR pszName2)
     }
 }
 
-static BOOL
+/* Do not assume that pch1 is terminated with UNICODE_NULL */
-CompareData(
+static BOOL MatchString(LPCWCH pch1, INT cch1, LPCWCH pch2, INT cch2)
-    DWORD   dwType,
-    LPCWSTR psz1,
-    LPCWSTR psz2)
 {
-    INT i, cch1 = wcslen(psz1), cch2 = wcslen(psz2);
+    INT i;
-    if (dwType == REG_SZ || dwType == REG_EXPAND_SZ)
+    DWORD dwNorm = ((s_dwFlags & RSF_MATCHCASE) ? NORM_IGNORECASE : 0);
-    {
-        if (s_dwFlags & RSF_WHOLESTRING)
-        {
-            if (s_dwFlags & RSF_MATCHCASE)
-                return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0,
-                                          psz1, cch1, psz2, cch2);
-            else
-                return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT,
-                                          NORM_IGNORECASE, psz1, cch1, psz2, cch2);
-        }
 
-        for(i = 0; i <= cch1 - cch2; i++)
+    if (s_dwFlags & RSF_WHOLESTRING)
-        {
+        return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1, cch1, pch2, cch2);
-            if (s_dwFlags & RSF_MATCHCASE)
+
-            {
+    if (cch1 < cch2)
-                if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0,
+        return FALSE;
-                                       psz1 + i, cch2, psz2, cch2))
+
-                    return TRUE;
+    for (i = 0; i <= cch1 - cch2; i++)
-            }
+    {
-            else
+        if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1 + i, cch2, pch2, cch2))
-            {
+            return TRUE;
-                if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT,
-                                       NORM_IGNORECASE, psz1 + i, cch2, psz2, cch2))
-                    return TRUE;
-            }
-        }
     }
+
+    return FALSE;
+}
+
+static BOOL MatchData(DWORD dwType, LPCVOID pv1, DWORD cb1)
+{
+    if (dwType == REG_SZ || dwType == REG_EXPAND_SZ || dwType == REG_MULTI_SZ)
+        return MatchString(pv1, (INT)(cb1 / sizeof(WCHAR)), s_szFindWhat, lstrlenW(s_szFindWhat));
+
     return FALSE;
 }
 
@@ -143,7 +134,7 @@ BOOL RegFindRecurse(
     LONG lResult;
     WCHAR szSubKey[MAX_PATH];
     DWORD i, c, cb, type;
-    BOOL fPast = FALSE;
+    BOOL fPast;
     LPWSTR *ppszNames = NULL;
     LPBYTE pb = NULL;
 
@@ -169,6 +160,7 @@ BOOL RegFindRecurse(
         goto err;
     ZeroMemory(ppszNames, c * sizeof(LPWSTR));
 
+    /* Retrieve the value names associated with the current key */
     for(i = 0; i < c; i++)
     {
         if (DoEvents())
@@ -192,10 +184,11 @@ BOOL RegFindRecurse(
 
     qsort(ppszNames, c, sizeof(LPWSTR), compare);
 
-    if (pszValueName == NULL)
+    /* If pszValueName is NULL, the function will search for all values within the key */
-        pszValueName = ppszNames[0];
+    fPast = (pszValueName == NULL);
 
-    for(i = 0; i < c; i++)
+    /* Search within the values */
+    for (i = 0; i < c; i++)
     {
         if (DoEvents())
             goto err;
@@ -212,10 +205,7 @@ BOOL RegFindRecurse(
                 CompareName(ppszNames[i], s_szFindWhat))
         {
             *ppszFoundSubKey = _wcsdup(szSubKey);
-            if (ppszNames[i][0] == 0)
+            *ppszFoundValueName = _wcsdup(ppszNames[i]);
-                *ppszFoundValueName = NULL;
-            else
-                *ppszFoundValueName = _wcsdup(ppszNames[i]);
             goto success;
         }
 
@@ -223,7 +213,7 @@ BOOL RegFindRecurse(
                                   NULL, &cb);
         if (lResult != ERROR_SUCCESS)
             goto err;
-        pb = malloc(cb + 3); /* To avoid buffer overrun, append 3 NULs */
+        pb = malloc(cb);
         if (pb == NULL)
             goto err;
         lResult = RegQueryValueExW(hSubKey, ppszNames[i], NULL, &type,
@@ -231,19 +221,10 @@ BOOL RegFindRecurse(
         if (lResult != ERROR_SUCCESS)
             goto err;
 
-        /* To avoid buffer overrun, append 3 NUL bytes.
+        if ((s_dwFlags & RSF_LOOKATDATA) && MatchData(type, pb, cb))
-           NOTE: cb can be an odd number although UNICODE_NULL is two bytes.
-           Two bytes at odd position is not enough to avoid buffer overrun. */
-        pb[cb] = pb[cb + 1] = pb[cb + 2] = 0;
-
-        if ((s_dwFlags & RSF_LOOKATDATA) &&
-                CompareData(type, (LPWSTR) pb, s_szFindWhat))
         {
             *ppszFoundSubKey = _wcsdup(szSubKey);
-            if (ppszNames[i][0] == 0)
+            *ppszFoundValueName = _wcsdup(ppszNames[i]);
-                *ppszFoundValueName = NULL;
-            else
-                *ppszFoundValueName = _wcsdup(ppszNames[i]);
             goto success;
         }
         free(pb);
@@ -258,6 +239,7 @@ BOOL RegFindRecurse(
     }
     ppszNames = NULL;
 
+    /* Retrieve the number of sub-keys */
     lResult = RegQueryInfoKeyW(hSubKey, NULL, NULL, NULL, &c, NULL, NULL,
                               NULL, NULL, NULL, NULL, NULL);
     if (lResult != ERROR_SUCCESS)
@@ -267,6 +249,7 @@ BOOL RegFindRecurse(
         goto err;
     ZeroMemory(ppszNames, c * sizeof(LPWSTR));
 
+    /* Retrieve the names of the sub-keys */
     for(i = 0; i < c; i++)
     {
         if (DoEvents())
@@ -290,6 +273,7 @@ BOOL RegFindRecurse(
 
     qsort(ppszNames, c, sizeof(LPWSTR), compare);
 
+    /* Search within the sub-keys */
     for(i = 0; i < c; i++)
     {
         if (DoEvents())
@@ -315,6 +299,7 @@ BOOL RegFindRecurse(
             goto success;
         }
 
+        /* Search within the value entries of the sub-key */
         if (RegFindRecurse(hSubKey, ppszNames[i], NULL, ppszFoundSubKey,
                            ppszFoundValueName))
         {
@@ -371,7 +356,7 @@ BOOL RegFindWalk(
     WCHAR szKeyName[MAX_PATH];
     WCHAR szSubKey[MAX_PATH];
     LPWSTR pch;
-    BOOL fPast;
+    BOOL fPast, fKeyMatched;
     LPWSTR *ppszNames = NULL;
 
     hBaseKey = *phKey;
@@ -470,7 +455,8 @@ BOOL RegFindWalk(
                 goto success;
             }
 
-            if (RegFindRecurse(hSubKey, ppszNames[i], NULL,
+            fKeyMatched = (_wcsicmp(ppszNames[i], szKeyName) == 0);
+            if (RegFindRecurse(hSubKey, ppszNames[i], (fKeyMatched ? pszValueName : NULL),
                                ppszFoundSubKey, ppszFoundValueName))
             {
                 LPWSTR psz = *ppszFoundSubKey;

base/applications/regedit/framewnd.c

@@ -383,7 +383,7 @@ static BOOL LoadHive(HWND hWnd)
                 /* refresh tree and list views */
                 RefreshTreeView(g_pChildWnd->hTreeWnd);
                 pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hRootKey);
-                RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath);
+                RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE);
             }
             else
             {
@@ -421,7 +421,7 @@ static BOOL UnloadHive(HWND hWnd)
         /* refresh tree and list views */
         RefreshTreeView(g_pChildWnd->hTreeWnd);
         pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hRootKey);
-        RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath);
+        RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE);
     }
     else
     {
@@ -518,7 +518,7 @@ static BOOL ImportRegistryFile(HWND hWnd)
     /* refresh tree and list views */
     RefreshTreeView(g_pChildWnd->hTreeWnd);
     pszKeyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hKeyRoot);
-    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, pszKeyPath);
+    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, pszKeyPath, TRUE);
 
     return bRet;
 }
@@ -877,7 +877,7 @@ static BOOL CreateNewValue(HKEY hRootKey, LPCWSTR pszKeyPath, DWORD dwType)
         return FALSE;
     }
 
-    RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath);
+    RefreshListView(g_pChildWnd->hListWnd, hRootKey, pszKeyPath, TRUE);
 
     /* locate the newly added value, and get ready to rename it */
     memset(&lvfi, 0, sizeof(lvfi));
@@ -885,7 +885,12 @@ static BOOL CreateNewValue(HKEY hRootKey, LPCWSTR pszKeyPath, DWORD dwType)
     lvfi.psz = szNewValue;
     iIndex = ListView_FindItem(g_pChildWnd->hListWnd, -1, &lvfi);
     if (iIndex >= 0)
+    {
+        ListView_SetItemState(g_pChildWnd->hListWnd, iIndex,
+                              LVIS_SELECTED | LVIS_FOCUSED, LVIS_SELECTED | LVIS_FOCUSED);
+        ListView_EnsureVisible(g_pChildWnd->hListWnd, iIndex, FALSE);
         (void)ListView_EditLabel(g_pChildWnd->hListWnd, iIndex);
+    }
 
     return TRUE;
 }
@@ -1128,11 +1133,11 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
     {
     case ID_EDIT_MODIFY:
         if (valueName && ModifyValue(hWnd, hKey, valueName, FALSE))
-            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE);
         break;
     case ID_EDIT_MODIFY_BIN:
         if (valueName && ModifyValue(hWnd, hKey, valueName, TRUE))
-            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+            RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE);
         break;
     case ID_EDIT_RENAME:
         if (GetFocus() == g_pChildWnd->hListWnd)
@@ -1180,7 +1185,7 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
                         item = ni;
                     }
 
-                    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+                    RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, FALSE);
                     if(errs > 0)
                     {
                         LoadStringW(hInst, IDS_ERR_DELVAL_CAPTION, caption, ARRAY_SIZE(caption));
@@ -1243,7 +1248,7 @@ static BOOL _CmdWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
     case ID_VIEW_REFRESH:
         RefreshTreeView(g_pChildWnd->hTreeWnd);
         keyPath = GetItemPath(g_pChildWnd->hTreeWnd, 0, &hKeyRoot);
-        RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath);
+        RefreshListView(g_pChildWnd->hListWnd, hKeyRoot, keyPath, TRUE);
         break;
         /*case ID_OPTIONS_TOOLBAR:*/
         /*	toggle_child(hWnd, LOWORD(wParam), hToolBar);*/

base/applications/regedit/listview.c

@@ -91,7 +91,7 @@ VOID SetValueName(HWND hwndLV, LPCWSTR pszValueName)
     {
         ListView_SetItemState(hwndLV, i, 0, LVIS_FOCUSED | LVIS_SELECTED);
     }
-    if (pszValueName == NULL)
+    if (pszValueName == NULL || pszValueName[0] == 0)
         i = 0;
     else
     {
@@ -101,6 +101,7 @@ VOID SetValueName(HWND hwndLV, LPCWSTR pszValueName)
     }
     ListView_SetItemState(hwndLV, i, LVIS_FOCUSED | LVIS_SELECTED,
                           LVIS_FOCUSED | LVIS_SELECTED);
+    ListView_EnsureVisible(hwndLV, i, FALSE);
     iListViewSelect = i;
 }
 
@@ -668,7 +669,7 @@ void DestroyListView(HWND hwndLV)
 
 }
 
-BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath)
+BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath, BOOL bSelectNone)
 {
     DWORD max_sub_key_len;
     DWORD max_val_name_len;
@@ -738,6 +739,9 @@ BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath)
     {
         ListView_SetItemState(hwndLV, i, 0, LVIS_FOCUSED | LVIS_SELECTED);
     }
+
+    if (bSelectNone)
+        iListViewSelect = -1;
     ListView_SetItemState(hwndLV, iListViewSelect,
                           LVIS_FOCUSED | LVIS_SELECTED,
                           LVIS_FOCUSED | LVIS_SELECTED);

base/applications/regedit/main.h

@@ -97,7 +97,7 @@ void ShowAboutBox(HWND hWnd);
 LRESULT CALLBACK ChildWndProc(HWND, UINT, WPARAM, LPARAM);
 void ResizeWnd(int cx, int cy);
 LPCWSTR get_root_key_name(HKEY hRootKey);
-VOID UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath);
+VOID UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone);
 
 /* edit.c */
 BOOL ModifyValue(HWND hwnd, HKEY hKey, LPCWSTR valueName, BOOL EditBin);
@@ -125,7 +125,7 @@ BOOL ExportRegistryFile(HWND hWnd);
 
 /* listview.c */
 HWND CreateListView(HWND hwndParent, HMENU id, INT cx);
-BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath);
+BOOL RefreshListView(HWND hwndLV, HKEY hKey, LPCWSTR keyPath, BOOL bSelectNone);
 WCHAR *GetValueName(HWND hwndLV, int iStartAt);
 BOOL ListWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result);
 BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result);

base/applications/regedit/treeview.c

@@ -643,7 +643,7 @@ BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result)
             /* Get the parent of the current item */
             HTREEITEM hParentItem = TreeView_GetParent(g_pChildWnd->hTreeWnd, pnmtv->itemNew.hItem);
 
-            UpdateAddress(pnmtv->itemNew.hItem, NULL, NULL);
+            UpdateAddress(pnmtv->itemNew.hItem, NULL, NULL, TRUE);
 
             /* Disable the Permissions menu item for 'My Computer' */
             EnableMenuItem(hMenuFrame, ID_EDIT_PERMISSIONS, MF_BYCOMMAND | (hParentItem ? MF_ENABLED : MF_GRAYED));
@@ -722,7 +722,7 @@ BOOL TreeWndNotifyProc(HWND hWnd, WPARAM wParam, LPARAM lParam, BOOL *Result)
                         lResult = FALSE;
                     }
                     else
-                        UpdateAddress(ptvdi->item.hItem, hRootKey, szBuffer);
+                        UpdateAddress(ptvdi->item.hItem, hRootKey, szBuffer, FALSE);
                 }
                 *Result = lResult;
             }