gdi32: first look for a printer driver name in the registry (try 2)

Vitaliy Margolen wine-devel at kievinfo.com
Tue Feb 22 21:16:44 CST 2011


On 02/22/2011 09:28 AM, Vitaly Perov wrote:
> Fixed possible hKey leak since last send
> 
> +    static const WCHAR user_printers_reg_key[] = { 'S','o','f','t','w','a','r','e','\\',
The actual key doesn't look like have anything to do with printers. Please
use variable name that more corresponds to the actual content of the
unicode string.

> +    if (!res && !RegQueryValueExW(hKey, device, NULL, NULL, (LPBYTE) driver, &size))
> +    {
> +        TRACE("Get value from registry\n");
> +    }
Message is not really helpful. And just hides empty if() case. It's better to
reverse logic to check for error and combine with the win.ini check. Something like:

    if ((RegOpenKeyExW(HKEY_CURRENT_USER, user_printers_reg_key, 0, KEY_READ, &hKey) ||
         RegQueryValueExW(hKey, device, NULL, NULL, (LPBYTE) driver, &size) &&
        !GetProfileStringW(devicesW, device, empty_strW, driver, size))

>          WARN("Unable to find %s in [devices] section of win.ini\n", debugstr_w(device));
This should also mention registry.

> +    if (!res) RegCloseKey(hKey);
Instead of using res, initialize hKey to NULL and close it if it's not NULL. It's
more foolproof.

Vitaliy



More information about the wine-devel mailing list