Re: [PATCH 2/2 v2] odbccp32: Implement SQLRemoveDriver/SQLRemoveDriverW

[Sebastian Lackner]

On 07.01.2016 08:15, Alistair Leslie-Hughes wrote:
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/odbccp32/odbccp32.c   | 57 ++++++++++++++++++++++++++++++++++++++++------
>  dlls/odbccp32/tests/misc.c |  4 ++--
>  2 files changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
> index 3d150a1..a55947b 100644
> --- a/dlls/odbccp32/odbccp32.c
> +++ b/dlls/odbccp32/odbccp32.c
> @@ -1105,22 +1105,65 @@ BOOL WINAPI SQLRemoveDefaultDataSource(void)
>      return FALSE;
>  }
>
> -BOOL WINAPI SQLRemoveDriverW(LPCWSTR lpszDriver, BOOL fRemoveDSN,
> -
>           LPDWORD lpdwUsageCount)
>
> +BOOL WINAPI SQLRemoveDriverW(LPCWSTR lpszDriver, BOOL removeDSN, LPDWORD usage_count)

When you are refactoring code like this, please also use "const WCHAR *" and "DWORD *" instead of LP* types.
Any reason why you keep the ugly name for lpszDriver?
> {
> - DWORD usage_cnt = 0;
> - HKEY hkey;
> - clear_errors();
> - FIXME("%s %d %p\n", debugstr_w(lpszDriver), fRemoveDSN, lpdwUsageCount);
> - if (lpdwUsageCount) *lpdwUsageCount = 1;
> - TRACE("%s %d %p\n", debugstr_w(lpszDriver), removeDSN, usage_count);
> -
> - if (RegOpenKeyW(HKEY_LOCAL_MACHINE, odbcini, &hkey) == ERROR_SUCCESS)
> - {
> -
>    HKEY hkeydriver;
> -
> -
>    if (RegOpenKeyW(hkey, lpszDriver, &hkeydriver) == ERROR_SUCCESS)
> -
>    {
> -
>        if (usage_count)
> -
>        {
> -
>            if (RegGetValueA(hkeydriver, NULL, "UsageCount", RRF_RT_DWORD, NULL, &usage_cnt, NULL) == ERROR_SUCCESS)
> -
>            {
> -
>                usage_cnt--;

Implementing UsageCount handling only here doesn't seem very useful. It also has to be set / updated,
and depending on the result the code below should behave different.
> -
>            }
> -
>        }
> -
> -
>        RegCloseKey(hkeydriver);
> -
>    }
> -
> -
>    if(RegDeleteKeyW(hkey, lpszDriver) != ERROR_SUCCESS)
> -
>        ERR("Failed to delete registry key: %s\n", debugstr_w(lpszDriver));
> -
> -
>    if (RegOpenKeyW(hkey, odbcdrivers, &hkeydriver) == ERROR_SUCCESS)
> -
>    {
> -
>        if(RegDeleteValueW(hkeydriver, lpszDriver) != ERROR_SUCCESS)
> -
>            ERR("Failed to delete registry value: %s\n", debugstr_w(lpszDriver));
> -
> -
>        RegCloseKey(hkeydriver);
> -
>    }
> -
> - }
> -
> - if (usage_count)
> -
>    *usage_count = usage_cnt;
> - return TRUE;
>
> }
>
> BOOL WINAPI SQLRemoveDriver(LPCSTR lpszDriver, BOOL fRemoveDSN,
>                 LPDWORD lpdwUsageCount)
>  {
> - WCHAR *driver;
> - BOOL ret;
> - clear_errors();
> - FIXME("%s %d %p\n", debugstr_a(lpszDriver), fRemoveDSN, lpdwUsageCount);
> - if (lpdwUsageCount) *lpdwUsageCount = 1;
> - return TRUE;
> - TRACE("%s %d %p\n", debugstr_a(lpszDriver), fRemoveDSN, lpdwUsageCount);
> -
> - driver = SQLInstall_strdup(lpszDriver);
> -
> - ret =  SQLRemoveDriverW(driver, fRemoveDSN, lpdwUsageCount);
> -
> - HeapFree(GetProcessHeap(), 0, driver);
> - return ret;
>
> }
>
> BOOL WINAPI SQLRemoveDriverManager(LPDWORD pdwUsageCount)
> diff --git a/dlls/odbccp32/tests/misc.c b/dlls/odbccp32/tests/misc.c
> index 3ae1fd3..e3adffd 100644
> --- a/dlls/odbccp32/tests/misc.c
> +++ b/dlls/odbccp32/tests/misc.c
> @@ -462,12 +462,12 @@ void test_SQLInstallDriverEx(void)
>      cnt = 100;
>      ret = SQLRemoveDriver("WINE ODBC Driver", FALSE, &cnt);
>      ok(ret, "SQLRemoveDriver failed\n");
> - todo_wine ok(cnt == 0, "SQLRemoveDriver failed %d\n", cnt);
> -
>
> ok(cnt == 0, "SQLRemoveDriver failed %d\n", cnt);
>
> cnt = 100;
> ret = SQLRemoveDriver("WINE ODBC Driver Path", FALSE, &cnt);
> ok(ret, "SQLRemoveDriver failed\n");
> - todo_wine ok(cnt == 0, "SQLRemoveDriver failed %d\n", cnt);
> - ok(cnt == 0, "SQLRemoveDriver failed %d\n", cnt);
>
> }
>
> START_TEST(misc)