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

On Thu, Jan 13, 2011 at 3:18 PM, Tor Lillqvist <tlillqvist@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice