Skip to content

Handle segfaults on startup#806

Merged
nlogozzo merged 19 commits into
cppfrom
cpp-morelogs
May 25, 2024
Merged

Handle segfaults on startup#806
nlogozzo merged 19 commits into
cppfrom
cpp-morelogs

Conversation

@JoseBritto
Copy link
Copy Markdown
Member

@JoseBritto JoseBritto commented May 23, 2024

Things handled:

  • Segfault when missing gresource file
  • Segfault when missing *.ui files
  • Segfault when locale not setup on system (im not entirely sure how to handle this, working on it atm)

Let me know if there are other things I missed, but these were mostly the problems I faced and having no logs made it harder to debug.

@JoseBritto JoseBritto changed the title Some logging in cases where Deanro segfaults on startup Handle segfaults on startup May 23, 2024
@nlogozzo
Copy link
Copy Markdown
Member

Segfault when locale not setup on system (im not entirely sure how to handle this, working on it atm)

You might have to take a look at libnick repository, as that is where all the localization happens. But it already handles if no local exists and defaults to en-us.

@JoseBritto
Copy link
Copy Markdown
Member Author

Segfault when locale not setup on system (im not entirely sure how to handle this, working on it atm)

You might have to take a look at libnick repository, as that is where all the localization happens. But it already handles if no local exists and defaults to en-us.

Oh. For me if I unset the locale, Denaro crashes. Basically std::locale("") throws runtime error causing a crash. How would I handle this?

@JoseBritto
Copy link
Copy Markdown
Member Author

In denaro there are two places that directly call std::locale. In the CurrencyHelpers::getSystemCurrency() method and the MainWindowController::getDebugInformation() method, I tried setting std::locale::global(std::locale("C")) but that doesn't seem to affect anything. I'm kinda lost here. Gtk by default falls back to this and produces an error:

(org.nickvision.money.gnome:27160): Gtk-WARNING **: 23:32:48.436: Locale not supported by C library.
	Using the fallback 'C' locale.

Is the correct way to have a small helper method/class that proxies all the calls to get the current locale and handle it there? Or is it better to handle the exception where ever we use std::locale directly?

@JoseBritto JoseBritto marked this pull request as ready for review May 23, 2024 23:57
@JoseBritto
Copy link
Copy Markdown
Member Author

In denaro there are two places that directly call std::locale. In the CurrencyHelpers::getSystemCurrency() method and the MainWindowController::getDebugInformation() method, I tried setting std::locale::global(std::locale("C")) but that doesn't seem to affect anything. I'm kinda lost here. Gtk by default falls back to this and produces an error:

(org.nickvision.money.gnome:27160): Gtk-WARNING **: 23:32:48.436: Locale not supported by C library.
	Using the fallback 'C' locale.

Is the correct way to have a small helper method/class that proxies all the calls to get the current locale and handle it there? Or is it better to handle the exception where ever we use std::locale directly?

Handled both separately.

@JoseBritto
Copy link
Copy Markdown
Member Author

Please let me know if I used the & and * or none at all for variables properly. I still get confused sometimes 😅
Everything worked when I tested.

@JoseBritto JoseBritto requested a review from nlogozzo May 24, 2024 00:01
Comment thread org.nickvision.money.gnome/src/application.cpp Outdated
Comment thread org.nickvision.money.gnome/src/application.cpp Outdated
Comment thread org.nickvision.money.gnome/src/application.cpp Outdated
Comment thread org.nickvision.money.gnome/src/helpers/builder.cpp Outdated
Comment thread org.nickvision.money.gnome/src/views/mainwindow.cpp Outdated
Comment thread libdenaro/src/helpers/currencyhelpers.cpp Outdated
Comment thread org.nickvision.money.gnome/src/application.cpp Outdated
@JoseBritto
Copy link
Copy Markdown
Member Author

All done

@JoseBritto JoseBritto requested a review from nlogozzo May 24, 2024 17:49
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/helpers/currencyhelpers.cpp Outdated
Comment thread libdenaro/src/helpers/currencyhelpers.cpp Outdated
Comment thread org.nickvision.money.gnome/src/helpers/builder.cpp Outdated
Comment thread org.nickvision.money.gnome/src/helpers/builder.cpp Outdated
Comment thread org.nickvision.money.gnome/src/application.cpp
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
Comment thread libdenaro/src/controllers/mainwindowcontroller.cpp Outdated
@JoseBritto
Copy link
Copy Markdown
Member Author

e777bf7 Added some more logs when opening and removing accounts. Let me know if I went a little overboard there.

@nlogozzo nlogozzo merged commit 6c08d80 into cpp May 25, 2024
@nlogozzo
Copy link
Copy Markdown
Member

e777bf7 Added some more logs when opening and removing accounts. Let me know if I went a little overboard there.

All is fine. Just only removed the Account removed one that had the account name, because i'd like to keep as little personal information as possible from the logs (at most file paths).

@nlogozzo nlogozzo deleted the cpp-morelogs branch May 25, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants