Date: prev next · Thread: first prev next last
2011 Archives by date, by thread · List index


Good for me, go for it!

On Thu, 2011-01-13 at 14:18 -0700, Tor Lillqvist wrote: 
So the problem must be in setup.exe, in some cases it fails to understand 
that it should use a transform. That is good, because it means we can debug it.

Which I did. Which was fun. Or at least entertaining. The code in 
libs-core/desktop/win32/source/setup/setup.cpp is, what shall I  say, amusing. It probably is 
based on code from Windows 3.1 era.

The problem was in the SetupAppX::GetProfileSection() method. This method loads one of the 
sections in the setup.ini file in the installer directory. It does some "clever" buffer 
management, initially allocating a relatively small buffer, and then growing it if the requested 
section doesn't fit. (With our multi-language installer, the [languages] section is quite large.)

To actually read the section, the Win32 function GetPrivateProfileSection() is used. 
Unfortunately the author of the code didn't read the documentation for that function's return 
value carefully enough. It says "If the buffer is not large enough to contain all the key name 
and value pairs associated with the named section, the return value is equal to nSize minus two". 
Alas, the code was written as if the return value would be the required size, if the passed 
buffer size was not big enough. (Which, sure, would be a saner API, and match many other Win32 
APIs.)

The first bug was in the test whether the return value indicated a too small buffer. The test 
never was true.

But if that was fixed, it didn't help, because the code that attempted to retry with a bigger 
buffer, instead of (for instance) multiplying the buffer size by two, because of the same 
misunderstanding, didn't increase it at all. Even if it would have increased it, with a fixed 
amount or multiplier, to be really really correct, surely it should loop, not increase it just 
once and retry.

So, the attached patch seems to fix the problem. The initial buffer size is now 10000 which de 
facto is big enough for us, but just in case, I kept the resizing (now as a loop) and in that the 
size is limited for sanity to a million characters or so. Should be enough. (And yes, I did 
verify that the looping resizing code works by temporarily using a much smaller initial buffer 
size.)

The symptom of the bug was that only the first 20 or so languages in the setup.ini file were 
recognized. German happens to be among those, which explains why it worked for Thalion72. 
Hungarian, Swedish and Portuguese (Brazil) are not, so they didn't work. In a test build with 
just a few languages, they all worked...

Please review the patch. Three approvals are needed to get this into the 3.3 installer.

--tml


_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice



Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.