On 2011-01-14 at 15:41, <nthiebaud@gmail.com> wrote:
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...
why [...] instead of [...]