Our existing tests for GetPrivateProfileString were not checking to see if strings were null terminated, and in particular, the case where lpDefault wasn't set, the code actually *didn't* set the target buffer to an empty string as it should. Valgrind noticed this problem, but the unit tests didn't, so I've updated the unit tests to add this test *and* fixed the relevant code to GetPrivateProfileString. Please find the patch attached.
--Chris
diff --git a/dlls/kernel32/profile.c b/dlls/kernel32/profile.c index 9b6b1a1..e52cd66 100644 --- a/dlls/kernel32/profile.c +++ b/dlls/kernel32/profile.c @@ -1114,9 +1114,10 @@ static int PROFILE_GetPrivateProfileString( LPCWSTR section, LPCWSTR entry, } else if (buffer && def_val) { lstrcpynW( buffer, def_val, len ); ret = strlenW( buffer ); - } - else + } else { + buffer[0] = 0; /* buffer must be a null terminated empty string */ ret = 0; + }
RtlLeaveCriticalSection( &PROFILE_CritSect );
diff --git a/dlls/kernel32/tests/profile.c b/dlls/kernel32/tests/profile.c index cddc9a0..2f0ff13 100644 --- a/dlls/kernel32/tests/profile.c +++ b/dlls/kernel32/tests/profile.c @@ -338,11 +338,17 @@ static void test_profile_existing(void) OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); ok(INVALID_HANDLE_VALUE != h, "%d: CreateFile failed\n",i); SetLastError(0xdeadbeef); + /* Fill the buffer with non-zero values so we can test if the + result is properly null terminated */ + memset(buffer, 0xff, sizeof(buffer)); ret = GetPrivateProfileStringA(SECTION, KEY, NULL, buffer, MAX_PATH, testfile2); if (!pe[i].read_error) ok( ret, "%d: GetPrivateProfileString failed with error %u\n", i, GetLastError() ); else + { ok( !ret, "%d: GetPrivateProfileString succeeded\n", i ); + ok( buffer[ret] == 0, "Get GetPrivateProfileString should terminate buffer with null\n" ); + } CloseHandle(h); } ok( DeleteFile(testfile2), "delete failed\n" );