James Hawkins wrote:
On Jan 21, 2008 2:14 PM, Christopher raccoonone@procyongames.com wrote:
Dmitry Timoshkov wrote:
"Dmitry Timoshkov" dmitry@codeweavers.com wrote:
It's not clear what this test is supposed to show. If the 1st call to LoadStringW is supposed to set resourcepointer to not NULL, why don't you test it? Then 'if(resourcepointer != NULL)' check and copying to copiedstring are not needed.
Also, if the test depends on a later patch to not fail, the test should be included in the patch.
Also, you need to test LoadStringA, to see if it behaves similarly. It would be also interesting to test LoadStringA/W with both buffer and buffer length set to 0.
In addition, as I already pointed out you need to inspect Wine source and fix the places which will be broken by your fix.
I tested LoadStringA under Windows XP, and calling it with buflen == 0 does not return a pointer to the resource. In fact LoadStringA seems to behave fairly differently from LoadStringW: in that calling with buffer == NULL causes an access violation instead of just returning 0.
That's why you need to add tests for LoadStringA to Wine's test suite.
How can I test for an access violation, won't that crash the test suite? Also, what's the guideline for what functions I need to write tests for when I send in a patch? I still don't understand why I need to write a test for LoadStringA since it has no dependence on LoadStringW (the function I'm patching).