Re: mlang/tests: don't assume output of ConvertStringFromUnicode is null terminated

2008-07-07 Thread James Hawkins
On Mon, Jul 7, 2008 at 12:08 AM, Dan Kegel [EMAIL PROTECTED] wrote:
 On Sun, Jul 6, 2008 at 10:29 AM, James Hawkins [EMAIL PROTECTED] 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).

-- 
James Hawkins




Re: mlang/tests: don't assume output of ConvertStringFromUnicode is null terminated

2008-07-06 Thread James Hawkins
2008/7/6 Dan Kegel [EMAIL PROTECTED]:
 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.

-- 
James Hawkins




Re: mlang/tests: don't assume output of ConvertStringFromUnicode is null terminated

2008-07-06 Thread Dan Kegel
On Sun, Jul 6, 2008 at 10:29 AM, James Hawkins [EMAIL PROTECTED] 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