Skip to content

Commit

Permalink
[REGEDIT] Fix ListView selection and finding (#5150)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
katahiromz committed Apr 1, 2023
1 parent aac8951 commit 84e580b
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 70 deletions.
4 changes: 2 additions & 2 deletions base/applications/regedit/childwnd.c
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
94 changes: 40 additions & 54 deletions base/applications/regedit/find.c
Expand Up @@ -87,41 +87,32 @@ static BOOL CompareName(LPCWSTR pszName1, LPCWSTR pszName2)
}
}

static BOOL
CompareData(
DWORD dwType,
LPCWSTR psz1,
LPCWSTR psz2)
/* Do not assume that pch1 is terminated with UNICODE_NULL */
static BOOL MatchString(LPCWCH pch1, INT cch1, LPCWCH pch2, INT cch2)
{
INT i, cch1 = wcslen(psz1), cch2 = wcslen(psz2);
if (dwType == REG_SZ || dwType == REG_EXPAND_SZ)
{
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);
}
INT i;
DWORD dwNorm = ((s_dwFlags & RSF_MATCHCASE) ? NORM_IGNORECASE : 0);

for(i = 0; i <= cch1 - cch2; i++)
{
if (s_dwFlags & RSF_MATCHCASE)
{
if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, 0,
psz1 + i, cch2, psz2, cch2))
return TRUE;
}
else
{
if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT,
NORM_IGNORECASE, psz1 + i, cch2, psz2, cch2))
return TRUE;
}
}
if (s_dwFlags & RSF_WHOLESTRING)
return 2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1, cch1, pch2, cch2);

if (cch1 < cch2)
return FALSE;

for (i = 0; i <= cch1 - cch2; i++)
{
if (2 == CompareStringW(LOCALE_SYSTEM_DEFAULT, dwNorm, pch1 + i, cch2, pch2, 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;
}

Expand All @@ -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;

Expand All @@ -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())
Expand All @@ -192,10 +184,11 @@ BOOL RegFindRecurse(

qsort(ppszNames, c, sizeof(LPWSTR), compare);

if (pszValueName == NULL)
pszValueName = ppszNames[0];
/* If pszValueName is NULL, the function will search for all values within the key */
fPast = (pszValueName == NULL);

for(i = 0; i < c; i++)
/* Search within the values */
for (i = 0; i < c; i++)
{
if (DoEvents())
goto err;
Expand All @@ -212,38 +205,26 @@ BOOL RegFindRecurse(
CompareName(ppszNames[i], s_szFindWhat))
{
*ppszFoundSubKey = _wcsdup(szSubKey);
if (ppszNames[i][0] == 0)
*ppszFoundValueName = NULL;
else
*ppszFoundValueName = _wcsdup(ppszNames[i]);
*ppszFoundValueName = _wcsdup(ppszNames[i]);
goto success;
}

lResult = RegQueryValueExW(hSubKey, ppszNames[i], NULL, &type,
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,
pb, &cb);
if (lResult != ERROR_SUCCESS)
goto err;

/* To avoid buffer overrun, append 3 NUL bytes.
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))
if ((s_dwFlags & RSF_LOOKATDATA) && MatchData(type, pb, cb))
{
*ppszFoundSubKey = _wcsdup(szSubKey);
if (ppszNames[i][0] == 0)
*ppszFoundValueName = NULL;
else
*ppszFoundValueName = _wcsdup(ppszNames[i]);
*ppszFoundValueName = _wcsdup(ppszNames[i]);
goto success;
}
free(pb);
Expand All @@ -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)
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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))
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
21 changes: 13 additions & 8 deletions base/applications/regedit/framewnd.c
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -877,15 +877,20 @@ 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));
lvfi.flags = LVFI_STRING;
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;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);*/
Expand Down
8 changes: 6 additions & 2 deletions base/applications/regedit/listview.c
Expand Up @@ -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
{
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions base/applications/regedit/main.h
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions base/applications/regedit/treeview.c
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 84e580b

Please sign in to comment.