[RFC PATCH 1/2] kernel32: Move implementation of {Get,Write}PrivateProfileStringW() to helper.
Hi folks, This series is trying to fix GetPrivateProfileStringA() to let it can deal with section\key\value with multibyte characters. The related real application is DISTRAINT: Deluxe Edition[1]. It saved localization information in some .ini files and used GetPrivateProfileStringA() for accessing them. The problem is MultiByteToWideChar(CP_ACP) in PROFILE_Load() will return decomposed string with '0x003f' for some multibyte characters when LC_CTYPE is zh_CN.UTF-8 or ja_JP.UTF-8. Then PROFILE_Load() will cache an wrong string and return a wrong result for GetPrivateProfileStringA(). But I'm not very sure if this is a good way to fix this issue since it hard coding codepage to 1252 for MultiByteToWideChar()/WideCharToMultiByte(). Any comments would be appreciated! [1] https://store.steampowered.com/app/395170/DISTRAINT_Deluxe_Edition/ Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/kernel32/profile.c | 97 ++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/dlls/kernel32/profile.c b/dlls/kernel32/profile.c index 027693e5da..462d64e5f0 100644 --- a/dlls/kernel32/profile.c +++ b/dlls/kernel32/profile.c @@ -1082,17 +1082,13 @@ UINT WINAPI GetProfileIntW( LPCWSTR section, LPCWSTR entry, INT def_val ) return GetPrivateProfileIntW( section, entry, def_val, wininiW ); } -/*********************************************************************** - * GetPrivateProfileStringW (KERNEL32.@) - */ -INT WINAPI GetPrivateProfileStringW( LPCWSTR section, LPCWSTR entry, - LPCWSTR def_val, LPWSTR buffer, - UINT len, LPCWSTR filename ) +static int get_private_profile_string(LPCWSTR section, LPCWSTR entry, LPCWSTR def_val, + LPWSTR buffer, UINT len, LPCWSTR filename) { - int ret; - LPWSTR defval_tmp = NULL; + int ret; + LPWSTR defval_tmp = NULL; - TRACE("%s,%s,%s,%p,%u,%s\n", debugstr_w(section), debugstr_w(entry), + TRACE("(%s, %s, %s, %p, %u, %s)\n", debugstr_w(section), debugstr_w(entry), debugstr_w(def_val), buffer, len, debugstr_w(filename)); /* strip any trailing ' ' of def_val. */ @@ -1114,22 +1110,25 @@ INT WINAPI GetPrivateProfileStringW( LPCWSTR section, LPCWSTR entry, } } - RtlEnterCriticalSection( &PROFILE_CritSect ); + RtlEnterCriticalSection(&PROFILE_CritSect); - if (PROFILE_Open( filename, FALSE )) { - if (section == NULL) + if (PROFILE_Open(filename, FALSE)) + { + if (section == NULL) ret = PROFILE_GetSectionNames(buffer, len); - else - /* PROFILE_GetString can handle the 'entry == NULL' case */ - ret = PROFILE_GetString( section, entry, def_val, buffer, len ); - } else if (buffer && def_val) { - lstrcpynW( buffer, def_val, len ); - ret = strlenW( buffer ); + else + /* PROFILE_GetString can handle the 'entry == NULL' case */ + ret = PROFILE_GetString(section, entry, def_val, buffer, len); + } + else if (buffer && def_val) + { + lstrcpynW(buffer, def_val, len); + ret = strlenW(buffer); } else - ret = 0; + ret = 0; - RtlLeaveCriticalSection( &PROFILE_CritSect ); + RtlLeaveCriticalSection(&PROFILE_CritSect); HeapFree(GetProcessHeap(), 0, defval_tmp); @@ -1138,12 +1137,23 @@ INT WINAPI GetPrivateProfileStringW( LPCWSTR section, LPCWSTR entry, return ret; } +/*********************************************************************** + * GetPrivateProfileStringW (KERNEL32.@) + */ +INT WINAPI GetPrivateProfileStringW(LPCWSTR section, LPCWSTR entry, + LPCWSTR def_val, LPWSTR buffer, + UINT len, LPCWSTR filename) +{ + return get_private_profile_string(section, entry, def_val, buffer, + len, filename); +} + /*********************************************************************** * GetPrivateProfileStringA (KERNEL32.@) */ -INT WINAPI GetPrivateProfileStringA( LPCSTR section, LPCSTR entry, - LPCSTR def_val, LPSTR buffer, - UINT len, LPCSTR filename ) +INT WINAPI GetPrivateProfileStringA(LPCSTR section, LPCSTR entry, + LPCSTR def_val, LPSTR buffer, + UINT len, LPCSTR filename) { UNICODE_STRING sectionW, entryW, def_valW, filenameW; LPWSTR bufferW; @@ -1159,9 +1169,9 @@ INT WINAPI GetPrivateProfileStringA( LPCSTR section, LPCSTR entry, if (filename) RtlCreateUnicodeStringFromAsciiz(&filenameW, filename); else filenameW.Buffer = NULL; - retW = GetPrivateProfileStringW( sectionW.Buffer, entryW.Buffer, - def_valW.Buffer, bufferW, len, - filenameW.Buffer); + retW = get_private_profile_string(sectionW.Buffer, entryW.Buffer, + def_valW.Buffer, bufferW, len, + filenameW.Buffer); if (len && buffer) { if (retW) @@ -1358,38 +1368,47 @@ INT WINAPI GetProfileSectionW( LPCWSTR section, LPWSTR buffer, DWORD len ) return GetPrivateProfileSectionW( section, buffer, len, wininiW ); } - -/*********************************************************************** - * WritePrivateProfileStringW (KERNEL32.@) - */ -BOOL WINAPI WritePrivateProfileStringW( LPCWSTR section, LPCWSTR entry, - LPCWSTR string, LPCWSTR filename ) +static BOOL write_private_profile_string(LPCWSTR section, LPCWSTR entry, LPCWSTR string, + LPCWSTR filename) { BOOL ret = FALSE; - RtlEnterCriticalSection( &PROFILE_CritSect ); + TRACE("(%s, %s, %s, %s)\n", debugstr_w(section), debugstr_w(entry), + debugstr_w(string), debugstr_w(filename)); + + RtlEnterCriticalSection(&PROFILE_CritSect); if (!section && !entry && !string) /* documented "file flush" case */ { - if (!filename || PROFILE_Open( filename, TRUE )) + if (!filename || PROFILE_Open(filename, TRUE)) { if (CurProfile) PROFILE_ReleaseFile(); /* always return FALSE in this case */ } } - else if (PROFILE_Open( filename, TRUE )) + else if (PROFILE_Open(filename, TRUE)) { if (!section) { SetLastError(ERROR_FILE_NOT_FOUND); } else { - ret = PROFILE_SetString( section, entry, string, FALSE); + ret = PROFILE_SetString(section, entry, string, FALSE); if (ret) ret = PROFILE_FlushFile(); } } - RtlLeaveCriticalSection( &PROFILE_CritSect ); + RtlLeaveCriticalSection(&PROFILE_CritSect); return ret; } +/*********************************************************************** + * WritePrivateProfileStringW (KERNEL32.@) + */ +BOOL WINAPI WritePrivateProfileStringW(LPCWSTR section, LPCWSTR entry, + LPCWSTR string, LPCWSTR filename) +{ + return write_private_profile_string(section, entry, string, + filename); +} + /*********************************************************************** * WritePrivateProfileStringA (KERNEL32.@) */ @@ -1408,8 +1427,8 @@ BOOL WINAPI DECLSPEC_HOTPATCH WritePrivateProfileStringA( LPCSTR section, LPCSTR if (filename) RtlCreateUnicodeStringFromAsciiz(&filenameW, filename); else filenameW.Buffer = NULL; - ret = WritePrivateProfileStringW(sectionW.Buffer, entryW.Buffer, - stringW.Buffer, filenameW.Buffer); + ret = write_private_profile_string(sectionW.Buffer, entryW.Buffer, + stringW.Buffer, filenameW.Buffer); RtlFreeUnicodeString(§ionW); RtlFreeUnicodeString(&entryW); RtlFreeUnicodeString(&stringW); -- 2.20.1
Jactry Zeng <jzeng(a)codeweavers.com> wrote:
This series is trying to fix GetPrivateProfileStringA() to let it can deal with section\key\value with multibyte characters. The related real application is DISTRAINT: Deluxe Edition[1]. It saved localization information in some .ini files and used GetPrivateProfileStringA() for accessing them. The problem is MultiByteToWideChar(CP_ACP) in PROFILE_Load() will return decomposed string with '0x003f' for some multibyte characters when LC_CTYPE is zh_CN.UTF-8 or ja_JP.UTF-8. Then PROFILE_Load() will cache an wrong string and return a wrong result for GetPrivateProfileStringA().
But I'm not very sure if this is a good way to fix this issue since it hard coding codepage to 1252 for MultiByteToWideChar()/WideCharToMultiByte().
That is certainly wrong. Very first thing to do (as usual) is to add a test case demonstarting the issue. -- Dmitry.
Jactry Zeng <jzeng(a)codeweavers.com> wrote:
That is certainly wrong. Very first thing to do (as usual) is to add a test case demonstarting the issue. There are some tests in [2/2].
These tests are either not convincing or broken: + lstrcpyA(expected_val, "\xe9\x85\x92\x2e"); + len = MultiByteToWideChar(CP_ACP, 0, expected_val, -1, expected_valW, MAX_PATH) - 1; You should not rely on CP_ACP being compatible with the multibyte string. + ret = GetPrivateProfileStringW(section_schineseW, name5W, NULL, + bufW, MAX_PATH, filenameW); + ok(ret == len || + (ret == 0 && bufW[0] == '\0') /* non-English */, "Expected %d, got %d\n", len, ret); A test that sometimes fails is broken. -- Dmitry.
On 2019/4/23 下午5:10, Dmitry Timoshkov wrote:
+ lstrcpyA(expected_val, "\xe9\x85\x92\x2e"); + len = MultiByteToWideChar(CP_ACP, 0, expected_val, -1, expected_valW, MAX_PATH) - 1;
You should not rely on CP_ACP being compatible with the multibyte string. This just want to get an expected result which corresponding to native GetPrivateProfileStringW()'s. + ret = GetPrivateProfileStringW(section_schineseW, name5W, NULL, + bufW, MAX_PATH, filenameW); + ok(ret == len || + (ret == 0 && bufW[0] == '\0') /* non-English */, "Expected %d, got %d\n", len, ret);
A test that sometimes fails is broken. OK, will improve this. Thanks.
Jactry Zeng <jzeng(a)codeweavers.com> wrote:
+ lstrcpyA(expected_val, "\xe9\x85\x92\x2e"); + len = MultiByteToWideChar(CP_ACP, 0, expected_val, -1, expected_valW, MAX_PATH) - 1;
You should not rely on CP_ACP being compatible with the multibyte string. This just want to get an expected result which corresponding to native GetPrivateProfileStringW()'s.
This can't work reliably, the test should be made to return reasonable results under different locales. -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Jactry Zeng