On 3/12/21 10:26 AM, Giovanni Mascellani wrote:
Hi,
thanks for the review!
Il 11/03/21 21:55, Nikolay Sivov ha scritto:
I don't see it crashing now anywhere on test vms. If that becomes an issue later on after implementation changes, this test diff should be included there.
It used to crash on my machine, because of a different set of installed fonts. I don't think the tests should be written just for the test VMs: any machine should reasonably be able to run them correctly and without crashes. The reason for this patch is that in C you should never dereference a NULL pointer, so the moment you get a pointer that might be NULL, you should test it before using it. If you want practical reasons, consider the case of a developer who wants to run the tests and has to waste their time chasing a segmentation fault caused by them having a different (but entirely legitimate) font set installed on their machine.
As far as I can tell this test is meant to show that explicit collection/font specified with MapCharacters() is actually used. It will only test if 'a' is supported in Tahoma, which should always be the case with wine's or native Tahoma. So unless you have a custom font named Tahoma without basic Latin support, which takes precedence over wine font, I don't understand why it fails for you. Could you explain?
In ideal test environment we would be using a set of specifically created fonts of course, so we don't depend on system fonts for tests where it's not desirable, but that might be a better fit for externally maintained test programs.
In other words, this patch makes the program more correct and gives less headaches to anybody using it, so why is it a problem?
It's not a problem, but I want to understand what's going on before suppressing failures or crashes.
Giovanni.