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

Max TenEyck Woodbury max at mtew.isa-geek.net
Sat May 4 13:59:17 CDT 2013


On 05/04/2013 11:30 AM, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury <max at mtew.isa-geek.net> wrote:
>
>>>> 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.
>>>
>>> First, what is needed, is an investigation what is happening exactly,
>>> that's what I asked about in the first place. If you belive that tmHeight
>>> being 0 is normal and should be accepted, then you need to provide more
>>> details and probably a test case.
>>>
>> Sorry, but that is not the case.  While an investigation could be
>> useful, the question that it is normal or not is not relevant.  The
>> simple fact is that it happens.  Since it DOES happen, the patch should
>> be applied, at least until some code elsewhere assures that it can not
>> happen.  If you want to throw in a 'FIXME' to the effect that it should
>> not happen and a test is needed to assure that, by all means do so.
>
> I wonder why did you bother with sending a patch at all? You don't want
> to provide any details or do any investigation. With such an attitude
> just open a bug report in bugzilla, and let somebody else do real work
> for you, but even then you will be asked to provide at least some details.
>
You are trying to make this about me.  It is not.  Windows fairly
obviously does not do this 'sanity' test.  Wine is supposed to imitate
windows.  To do this absolutely correctly, the whole 'sanity' test
should go away.

The patch merely reduces the problem from totally unacceptable to
tolerable.  The original code snippet is low quality.  There should not
be any division in the 'sanity' test.  As I said, it really does not
belong in wine.  If you insist on including it in something like its
original form, multiply both sides of the comparison by the
'denominator'.  Maybe throw in a couple of 'iabs(...)'s.

Why send the patch?  Because, for me, all versions of wine have been
unacceptable since the problem appeared.  Wine simply should not throw
this exception.  An application that uses a font with this problem will
blow up.  The standard 'make test' suite on my machine has the problem.

I isolated the problem, and came up with a fix.  Bug reports are for
cases where you can not yourself identify the bad code.  That this code
is bad is obvious when you know that it can throw an exception.  The
only investigation absolutely needed is to report the occurrence of the
exception.  It happens in at least some circumstances.  Anything
additional is simply an invitation to delay.




More information about the wine-devel mailing list