[PATCH] dlls/gdi32/fretype.c: Avoid division by zero.

Max TenEyck Woodbury max at mtew.isa-geek.net
Sat May 4 07:09:14 CDT 2013


On 05/04/2013 01:56 AM, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury <max at mtew.isa-geek.net> wrote:
>
>>>> +        if ( font->aveWidth && font->potm->otmTextMetrics.tmHeight ) {
>>>> +            if (((font->aveWidth + font->potm->otmTextMetrics.tmHeight - 1) /
>>>> +                 font->potm->otmTextMetrics.tmHeight) > 100) {
>>>>                    WARN("Ignoring too large font->aveWidth %d\n", font->aveWidth);
>>>>                    font->aveWidth = 0;
>>>>                }
>>>
>>> In which case font->potm->otmTextMetrics.tmHeight is going to be 0?
>>>
>> I am not sure what you are asking.  I have had this particular division
>> throw an exception for some font I have installed, but I have no idea
>> which one at the moment.  So if you are implying that font->aveWidth==0
>> is always true if font->potm->otmTextMetrics.tmHeight==0, that seems not
>> to be the case.
>
> If font->potm->otmTextMetrics.tmHeight is 0 then the font is invalid and
> should not be loaded and used at all. There is no point is adding checks
> for things that shouldn't happen, and may cause various bad things in other
> places of code as well.
>
Sorry, but WRONG.  It HAPPENS.  Someplace in the collection of fonts I
have accumulated from standard sources, there is at least one that has
this property but is otherwise useful.  You can say all you want about
'it should not be loaded', but it does load.  This deals with the
problem.

Further, this was NOT a problem until the patch that introduced the
SCALE_X and SCALE_Y macros.  I did a bisection and that was the commit
that introduced the divide by zero exception.  Before that patch, there
was no exception thrown by this code.  With that patch and since then
it throws a divide by zero consistently.  I did NOT change my font
mix.  The wine test suite simply started throwing a divide by zero
exception at that point.  I have not gone over that patch in detail,
but its intent seems to have been clear.  There was just some detail
that messed up.  This patch corrects the problem.

If you REALLY think the font should not load, you should add code to
reject the font with an appropriate diagnostic, not have this code
throw a divide by zero exception and abort execution.  Until you do
that, this patch is needed.



More information about the wine-devel mailing list