On 07/23/2010 08:57 AM, Vitaliy Margolen wrote:
On 07/21/2010 10:23 AM, Andrew Eikum wrote:
Please more-less follow the format of the file and don't add yet one more.
+ if(outLen == 0)
+ return 0;
Need space between "if" and "(".
+ if(!buf){
Curly bracket goes on to separate line.
You're right I should've checked the entire file. Because Wine's
formatting is so inconsistent already, I usually just bother to follow
the immediate function's standards, which in this case, there was none.
+ buf = HeapAlloc(GetProcessHeap(), 0, outLen * sizeof(WCHAR));
+ ret = GetPrivateProfileStringW(appName, keyName, NULL, buf, outLen,
filename);
+ strcpyW(out, buf);
+ HeapFree(GetProcessHeap(), 0, buf);
Why do you need extra buffer here? Use passed buffer instead.
GetPrivateProfileString with NULL keyName fills the buffer with all of
the available keys, separated by nuls. This shlwapi function only fills
the buffer with the first available key, leaving the rest of the buffer
unmodified. I had tests for this, but must've forgotten them when I
rewrote the tests before submitting. Attached patch demonstrates.
Thanks for reviewing,
Andrew
diff --git a/dlls/shlwapi/tests/ordinal.c b/dlls/shlwapi/tests/ordinal.c
index d60effd..4a8a14a 100644
--- a/dlls/shlwapi/tests/ordinal.c
+++ b/dlls/shlwapi/tests/ordinal.c
@@ -2598,11 +2598,13 @@ static void test_SHGetIniString(void)
ok(ret == 0, "SHGetIniStringW should have given 0, instead: %d\n", ret);
/* valid arguments */
+ memset(out, 0, sizeof(out));
ret = pSHGetIniStringW(TestAppW, NULL, out, sizeof(out), TestIniW);
ok(broken(ret == 0) || /* win 98 */
ret == 4, "SHGetIniStringW should have given 4, instead: %d\n",
ret);
ok(!lstrcmpW(out, AKeyW), "Expected %s, got: %s\n",
wine_dbgstr_w(AKeyW), wine_dbgstr_w(out));
+ ok(*(out + lstrlenW(AKeyW) + 1) == 0, "Should've been NULLs after first
key\n");
ret = pSHGetIniStringW(TestAppW, AKeyW, out, sizeof(out), TestIniW);
ok(broken(ret == 0) || /* win 98 */
