http://bugs.winehq.org/show_bug.cgi?id=19759
--- Comment #7 from Juan Lang juan_lang@yahoo.com 2009-08-17 19:33:11 --- (In reply to comment #6)
Change to SLTG_DoVars, strictly speaking, is not required given that Wine's implementation of SysAllocStringLen is zero-filling the buffer. However, I think the change is good to have, as this behaviour is not guaranteed (by other implementations?).
On one hand, I think you're right that this is consistent. I was wondering whether MultiByteToWideChar might already be NULL-terminating, but that was only based on a cursory read. Since you're passing an explicit source string length to it, it doesn't guarantee that the destination is NULL terminated. So explicitly NULL terminating it does make the expectation clear.
On the other hand, since you're changing one part of oleaut32, it's impossible to be using a different implementation of SysAllocStringLen at the same time.
As usual, a test case would resolve these difficulties once and for all, but writing one may be impossible: the buffer returned by SysAllocStringLen could depend on the compiler and flags used by Microsoft when building oleaut32. While perhaps the buffer isn't guaranteed to be set entirely to be zero, there is one test at least that checks that an empty string (length 0) has a NULL terminator in it [1]. Since the test passes consistently across a wide variety of Windows platforms [2], I'd say it's at least suggestive that while Microsoft doesn't _claim_ to provide a zero buffer, in fact it does, or at the very least a NULL-terminated one.
In summary, I'd suggest you keep the first change but omit the second: the second really does seem superfluous, and while in principle it can't hurt, it might raise objections that would keep your patch out longer than necessary.
[1] http://source.winehq.org/source/dlls/oleaut32/tests/vartype.c#L5229 [2] http://test.winehq.org/data/tests/oleaut32:vartype.html