[PATCH] dlls/gdi32/fretype.c: Avoid division by zero.
Sam Edwards
cfsworks at gmail.com
Sat May 4 16:37:14 CDT 2013
On 05/04/2013 12:59 PM, Max TenEyck Woodbury wrote:
> 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.
This sounds like an excellent reason to write a conformance test. A test
that succeeds on Windows but fails under Wine is a great way to convince
everyone that the patch is necessary. It's also gives us a closer look
at Windows's behavior under the same circumstances, so we can see
whether Windows does this sanity check or not, and if not, how it reacts
when aveWidth is wrong.
I should note, the commit that introduces the sanity check
(21589993826cdb1cb2f87ceb94c8a188bd4a3090) also includes a test
(dlls/gdi32/tests/font.c:3908 as of this writing) that passes under
Windows, which could mean that Windows actually does include this sanity
check for the width vs. the height.
> 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.
Are we sure that *this* code is the problem? As Dmitry has said,
tmHeight should never be 0, so it's probably valid to assume
tmHeight!=0. The bug may instead be in allowing the font to load with
tmHeight=0, and this is merely the first crash that the problem causes
for you. But what about apps that divide by tmHeight under the same
assumption? We can't change those, so it's better to fix tmHeight.
Are delays necessarily a bad thing? This bug doesn't have any security
implications, and we aren't hurrying to catch the Wine 1.6 release
window. We can afford to take the extra time to ensure the quality of
the patch. :)
More information about the wine-devel
mailing list