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 <leslie_alistair(a)hotmail.com>
> ---
>  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)