Szymon Acedanski wants to merge 1 commit from /u/accek/flightgear/ to next, 2017-03-11
This is happening to me intermittently during initialization, when compiled in debug variant and I suspect some relationship to the new splash screen.
Nevertheless FG should not crash because of a missing locale string. The same code as I add in this commit is already there for currentLocale above the changed lines.
Commit | Date | |
---|---|---|
[1fc358]
(crash-def-locale)
by
Szymon Acedański
Fixed crash with string not present in the default locale |
2017-03-07 20:23:22 | Tree |
Ah, this explains the preview fix :) Okay this should also never occur, we need to do some deeper debugging here since this code should be safe without the extra guard. Can you give some idea of frequency? And it's only in debug mode?
By "intermittently" I mean something like 2 out of 3 times when it happens at all.
I saw it only when compiled as Debug target.
Attaching the crash log.
Unfortunately, I can't reproduce it reliably even in Debug right now. I'll try to gather some more forensics once this happens again.
Hi,
Things work fine for me with LANG=en_US.UTF-8 as well as LANG=fr_FR.UTF-8 (with the latter, I see the splash screen tips in English, which is expected since $FG_ROOT/Translations/fr/tips.xml doesn't exist).
It seems to me the crash you are reporting could happen if FGData is older than:
commit c8dcc932862889c0c3cfcc37494fdb7599710806
Date: Sun Feb 26 21:59:47 2017 +0000
Initial examples of startup tips.
which declares the 'tips' resource for the default locale in Translations/locale.xml (loaded into /sim/intl/locale[0]/strings/tips).
-> are you sure you had FGData more recent than that when you saw the crash?
Right, that's probably the case. Sorry for the noise.
I still think this one should go in, as in any case a crash with touching random memory is not a good behavior and other methods: getLocalizedString, getLocalizedStringWithIndex, getLocalizedStrings handle missing translations gracefully. What do you think?
In principle, I'd say yes. But since James wrote the code and seems to have a precise idea of what should and should not work, let's see what he thinks. :)
The rationale is, that /all/ the locale code assumes the default locale is present - we error out during startup if this is not the case. So after that point, it is safe to assume the default locale is loaded.
We can adjust all the locale code to tolerate a missing default locale, but we have excluded that possibility during startup, so I don't see it as a high priority. It will cause a cascade of 'deal with empty string' changes in various places.
This makes sense, as a way to enhance code readability. Szymon, are you convinced?..
Ok, sounds good. Dropping this one, too, then.