[Libreoffice] (un)localized MSI installer [REVIEW REQUEST]

[Norbert Thiebaud]

On Thu, Jan 13, 2011 at 3:18 PM, Tor Lillqvist <tlillqvist at novell.com> 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.

why a:

f(x)
while()
{
    f(x)
}


instead fo a

do
{
    f(x)
}
while()

?

Norbert



>
> --tml
>
>
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice
>
>