Re: dlls/userenv: fixed stubs GetUserProfileDirectoryW/A

[Nikolay Sivov]

Andreas Rosenberg wrote:

+    TRACE("%p %s %p\n", hToken,  debugstr_w(lpProfileDir), lpcchSize );
+    /* profile specific tokens not supported, so hToken ignored  */
+    
+    SetLastError(0);
+    res = RegOpenKeyExW(HKEY_LOCAL_MACHINE,profile_pathname,0L,KEY_QUERY_VALUE,&keyProfileDir);

Why do you need this SetLastError(0)?

+        res = RegGetValueW(keyProfileDir,profile_subkey,profile_keyname,RRF_RT_ANY,
+                           NULL,buffer,&sizePath);  /* RegGetValue expects pcbData in bytes */
+        error = GetLastError();
+        RegCloseKey(keyProfileDir);

error isn't used after that.

+            /* else ("GetUserNameW failed\n"); */
+        }
+        /* else ("RegGetValueW failed - %d %d %d\n",res,error,sizePath); */
+    }
+    /* else ("RegOpenKeyExW failed\n"); */
     return FALSE;

What do these debug-comment lines mean?

dlls/userenv: fixed stubs GetUserProfileDirectoryW/A

[Andreas Rosenberg]

On Monday 02 March 2009 17:26:47 Nikolay Sivov wrote:
> Andreas Rosenberg wrote:
> - TRACE("%p %s %p\n", hToken,  debugstr_w(lpProfileDir), lpcchSize );
> - /* profile specific tokens not supported, so hToken ignored  */
> -
> - SetLastError(0);
> - res =
>
> RegOpenKeyExW(HKEY_LOCAL_MACHINE,profile_pathname,0L,KEY_QUERY_VALUE,&keyPr
> ofileDir);
>
> Why do you need this SetLastError(0)?

Shouldn't every API function clear this? Otherwise your will get an error 
state, from a previous API call when checking GetLastError which may
result in strange error codes.
> -
>    res =
>
> RegGetValueW(keyProfileDir,profile_subkey,profile_keyname,RRF_RT_ANY, +    
>                       NULL,buffer,&sizePath);  /* RegGetValue expects
> pcbData in bytes */ +        error = GetLastError();
> -
>    RegCloseKey(keyProfileDir);
>
> error isn't used after that.
> -
>        /* else ("GetUserNameW failed\n"); */
> -
>    }
> -
>    /* else ("RegGetValueW failed - %d %d %d\n",res,error,sizePath);
>
> */ +    }
> - /* else ("RegOpenKeyExW failed\n"); */
> return FALSE;
>
> What do these debug-comment lines mean?

I had added more code here to show debug information. I left it in case
some later debugging should be required.

dlls/userenv: fixed stubs GetUserProfileDirectoryW/A

[Nikolay Sivov]

Andreas Rosenberg wrote:
> On Monday 02 March 2009 17:26:47 Nikolay Sivov wrote:
> > Andreas Rosenberg wrote:
> > - TRACE("%p %s %p\n", hToken,  debugstr_w(lpProfileDir), lpcchSize );
> > - /* profile specific tokens not supported, so hToken ignored  */
> > -
> > - SetLastError(0);
> > - res =
> >
> > RegOpenKeyExW(HKEY_LOCAL_MACHINE,profile_pathname,0L,KEY_QUERY_VALUE,&keyPr
> > ofileDir);
> >
> > Why do you need this SetLastError(0)?
>
> Shouldn't every API function clear this? Otherwise your will get an error 
> state, from a previous API call when checking GetLastError which may
> result in strange error codes.

Of course it shouldn't clear it. You should set last error only when 
your call exits with an error. Even in this case
not every Win call set it reliably (and this could easily change between 
Win versions).
You should add a testcase to use SetLastError properly.
In your case res should contain an error code already (no GetLastError 
needed).
> > -
> >    res =
> >
> > RegGetValueW(keyProfileDir,profile_subkey,profile_keyname,RRF_RT_ANY, +    
> >                       NULL,buffer,&sizePath);  /* RegGetValue expects
> > pcbData in bytes */ +        error = GetLastError();
> > -
> >    RegCloseKey(keyProfileDir);
> >
> > error isn't used after that.
> > -
> >        /* else ("GetUserNameW failed\n"); */
> > -
> >    }
> > -
> >    /* else ("RegGetValueW failed - %d %d %d\n",res,error,sizePath);
> >
> > */ +    }
> > - /* else ("RegOpenKeyExW failed\n"); */
> > return FALSE;
> >
> > What do these debug-comment lines mean?
>
> I had added more code here to show debug information. I left it in case
> some later debugging should be required.

Simply don't left anything unused in a current patch. You can always add 
this later.