2008/7/6 Dan Kegel dank@kegel.com:
The mlang test assumed that the output of ConvertStringFromUnicode was null terminated, but it seems not to be.
Fixes the valgrind warning: Conditional jump or move depends on uninitialised value(s) at strlen (mc_replace_strmem.c:242) by lstrlenA (string.c:364) by ConvertINetMultiByteToUnicode (mlang.c:526) by ConvertINetString (mlang.c:633) by fnIMultiLanguage2_ConvertString (mlang.c:2197) by check_convertible (mlang.c:287) by test_EnumCodePages (mlang.c:407) Uninitialised value was created by a stack allocation at check_convertible (mlang.c:270)
I believe James was trying to fix this with http://www.winehq.org/pipermail/wine-patches/2008-June/056454.html but missed (he thought the problem was on the destination len, but really it was on the source len?).
No, this is hiding a bug. The test code conforms with the API. The problem is that ConvertINetMultiByteToUnicode uses the value of an out-only parameter (NULL pDstStr, non-NULL pcDstSize). Check out the code block in mlang.c:632.
On Sun, Jul 6, 2008 at 10:29 AM, James Hawkins truiken@gmail.com wrote:
No, this is hiding a bug. The test code conforms with the API. The problem is that ConvertINetMultiByteToUnicode uses the value of an out-only parameter (NULL pDstStr, non-NULL pcDstSize). Check out the code block in mlang.c:632.
Yeah, that's what I figured you thought, but your fix doesn't actually get rid of the error message, and the error is happening on *pcSrcSize = lstrlenA(pSrcStr); in ConvertINetMultiByteToUnicode.
The conformance test shows on line 197 that IMultiLanguage2_ConvertStringFromUnicode doesn't null-terminate its output. So it's wrong for check_convertible to rely on it to do so.
Am I missing something? I still believe in my patch... - Dan
On Mon, Jul 7, 2008 at 12:08 AM, Dan Kegel dank@kegel.com wrote:
On Sun, Jul 6, 2008 at 10:29 AM, James Hawkins truiken@gmail.com wrote:
No, this is hiding a bug. The test code conforms with the API. The problem is that ConvertINetMultiByteToUnicode uses the value of an out-only parameter (NULL pDstStr, non-NULL pcDstSize). Check out the code block in mlang.c:632.
Yeah, that's what I figured you thought, but your fix doesn't actually get rid of the error message, and the error is happening on *pcSrcSize = lstrlenA(pSrcStr); in ConvertINetMultiByteToUnicode.
The conformance test shows on line 197 that IMultiLanguage2_ConvertStringFromUnicode doesn't null-terminate its output. So it's wrong for check_convertible to rely on it to do so.
Am I missing something? I still believe in my patch...
The thing that's wrong with your patch is that you're mixing source size and dest size, which aren't necessarily similar values depending on code page. The right fix is to make sure the NULL terminator is encoded too in the first conversion. I also fixed up ConvertINetMultiByteToUnicode while I was at it (accessing out-only param).
-
Dan Kegel -
James Hawkins