Menu

FlightGear Merge Request #81: Fixed crash with string not present in the default locale (rejected)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

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 Szymon Acedański

Fixed crash with string not present in the default locale

2017-03-07 20:23:22 Tree

Discussion

  • James Turner

    James Turner - 2017-03-10

    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?

     
  • Szymon Acedanski

    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.

     
  • Florent Rougon

    Florent Rougon - 2017-03-11

    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?

     
  • Szymon Acedanski

    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?

     
  • Florent Rougon

    Florent Rougon - 2017-03-11

    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. :)

     
  • James Turner

    James Turner - 2017-03-11

    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.

     
  • Florent Rougon

    Florent Rougon - 2017-03-11

    This makes sense, as a way to enhance code readability. Szymon, are you convinced?..

     
  • Szymon Acedanski

    Ok, sounds good. Dropping this one, too, then.

     
  • Szymon Acedanski

    • Status: open --> rejected
     

Log in to post a comment.