On 05/04/2013 11:30 AM, Dmitry Timoshkov wrote:
Max TenEyck Woodbury max@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.