Jeff L wrote:
Change log: Add ScriptTextOut functionality and restructure tests.
This patch compiles with warnings on my machine:
usp10.c: In function ‘test_ScriptTextOut’: usp10.c:442: warning: passing argument 6 of ‘ScriptTextOut’ from incompatible pointer type usp10.c:360: warning: unused variable ‘cnt’ ../../../tools/winegcc/winegcc -B../../../tools/winebuild -mconsole usp10.o testlist.o -o usp10_test.exe
It also causes the test cases to crash. I see you may have addressed that in the next patch, but each patch should be valid on its own, so that there's no points in the commit history where Wine won't compile, or the tests fail.
fixme:uniscribe:ScriptGetCMap ((nil),(nil),(null),0,0x0,(nil)): semi-stub wine: Unhandled page fault on read access to 0x00000000 at address 0x7fe1d446 (thread 0009), starting debugger... WineDbg starting on pid 0x8 Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x7fe1d446). Register dump:
In the test, you've moved the functions around, which makes it difficult to see what you've added and removed.
-void test_ScriptGetFontProperties(void)
+void test_ScriptGetFontProperties(void)
It makes things easier to review if you avoid moving code unnecessarily
thanks,
Mike
Mike McCormack wrote:
It also causes the test cases to crash. I see you may have addressed that in the next patch, but each patch should be valid on its own, so that there's no points in the commit history where Wine won't compile, or the tests fail.
I'll combine them again as I think that it is too hard to tease the bits out. Thought that was clean break but alas.
In the test, you've moved the functions around, which makes it difficult to see what you've added and removed.
It makes things easier to review if you avoid moving code unnecessarily
This was unfortunate but it seems to be the result of moving code above ScriptGetFontProperties and diff decided to show this the code was deleted and readded. The bulk of the code is the same but I don't know how you restructure without it being moved around.
thanks,
Mike
Jeff