What is wrong with my patch?
I have fixed all errors found by Juan Lang. I resent this patch several times, but hasn't received any answer.
Why my patch is ignored?
Maybe something is still wrong, but why somebody just tell me about it?
"Juan Lang" juan.lang@gmail.com wrote:
Hi Vitaliy, overall your patch looks pretty good. A few minor points:
- FIXME("(%s %d %p) partially implemented\n",
debugstr_w(servername), level, buf); Because you add a more specific FIXME for every unimplemented path, it seems to me this would be better as a TRACE.
- result = RegCreateKeyExA(HKEY_LOCAL_MACHINE, regKey, 0, NULL, 0,
KEY_WRITE,
- NULL, &hKey, &disposition);
- if (result != ERROR_SUCCESS)
- {
- FIXME("Unable to open/create key 'HKLM\%s'\n", regKey);
- return result;
This shouldn't be a FIXME, as there's nothing to "fix" in the code: if the key couldn't be created, there's a setup problem or some other unrecoverable error. A TRACE is fine.
- /* FIXME: A duplicate patch checking should be implemented here */
Do you mean a duplicate path?
Vitaly Perov vitperov@etersoft.ru writes:
- GetCurrentDirectoryA(MAX_PATH, tmpPath);
- result = SetCurrentDirectoryW(path);
- SetCurrentDirectoryA(tmpPath);
- if (!result) return ERROR_FILE_NOT_FOUND;
This is not the right way to test if a directory exists.
On Monday 01 December 2008 18:45:22 Alexandre Julliard wrote:
Vitaly Perov vitperov@etersoft.ru writes:
- GetCurrentDirectoryA(MAX_PATH, tmpPath);
- result = SetCurrentDirectoryW(path);
- SetCurrentDirectoryA(tmpPath);
- if (!result) return ERROR_FILE_NOT_FOUND;
This is not the right way to test if a directory exists.
Thank you for your answer. I will fix it.
Vitaly Perov wrote:
What is wrong with my patch?
I have fixed all errors found by Juan Lang. I resent this patch several times, but hasn't received any answer.
Why my patch is ignored?
Maybe something is still wrong, but why somebody just tell me about it?
"Juan Lang" juan.lang@gmail.com wrote:
Hi Vitaliy, overall your patch looks pretty good. A few minor points:
- FIXME("(%s %d %p) partially implemented\n",
debugstr_w(servername), level, buf); Because you add a more specific FIXME for every unimplemented path, it seems to me this would be better as a TRACE.
- result = RegCreateKeyExA(HKEY_LOCAL_MACHINE, regKey, 0, NULL, 0,
KEY_WRITE,
NULL, &hKey, &disposition);
- if (result != ERROR_SUCCESS)
- {
FIXME("Unable to open/create key 'HKLM\\%s'\n", regKey);
return result;
This shouldn't be a FIXME, as there's nothing to "fix" in the code: if the key couldn't be created, there's a setup problem or some other unrecoverable error. A TRACE is fine.
- /* FIXME: A duplicate patch checking should be implemented here */
Do you mean a duplicate path?
I'm not sure about this, but I don't like the mixture of A and W-calls.