gdiplus: Add test cases for GdipDrawCurve [corrected]

Andrew Eikum andrew at brightnightgames.com
Sat Jun 6 10:20:06 CDT 2009


Nikolay Sivov wrote:
> Paul Vriens wrote:
>> Andrew Eikum wrote:
>>> This patch was submitted back on Tuesday and I haven't received a 
>>> response one way or the other about it.  Does anyone see anything 
>>> immediately wrong with it?
>>>
>>> I more-or-less copied the functionality of test_GdipDrawBezier right 
>>> above, testing each of the different input possibilities for 
>>> correctness.  It passes 100% on WinXP SP3 and Win7 RC1, although 
>>> there are failures in Wine's GdipDrawCurve implementation.
>>>
>>> Thanks for taking a look,
>>> Andrew
>>>
>>>
>> Hi Andrew,
>>
>> Test crashes on my box:
>>
>> Not sure if that was the reason for not being committed though as the 
>> tests could run fine on AJ's magic box of course.
>>
> +    /* InvalidParameter cases: invalid count */
> +    status = GdipDrawCurve(graphics, pen, points, -1);
> +    expect(InvalidParameter, status);
> +
> +    status = GdipDrawCurve(graphics, pen, points, 0);
> +    expect(InvalidParameter, status);
>
> This could be a problem. Count isn't checked on allocation in 
> GdipDrawCurve2(), and allocated buffer isn't checked for NULL too.
Well, even if the tests cause it to crash, doesn't that make this a 
problem with GdipDrawCurve2 and not the tests patch?  The tests do not 
crash on Windows, which means there's nothing wrong with the tests 
themselves.  If tests are not ever supposed to crash Wine like this, 
wouldn't it be more appropriate to make a test harness that catches all 
exceptions and reports as failures?  I'm just not seeing how this crash 
is the _tests'_ fault.

In any case, the problem is as you described.  The return of GdipAlloc 
isn't checked, causing a crash when it tries to access the structures it 
expects to be there.  Would these tests be accepted if there was an 
accompanying patch to fix this defect, eliminating the crash?



More information about the wine-devel mailing list