Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands
We still have problems with logging std::cerr on Windows, so I prefer keeping it in std::cout for now. I have tested this by leaving Widelands while another terminal was linking a debug build - that will make my system slow enough to trigger it *lol Thanks for the review! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1794924-logger-assert/+merge/355823 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands
Diff is looking okay, but unfortunately I am unable to force the message on purpose so I wasn't able to test it. As a suggestion: Maybe use std::cerr instead of std::cout for this message? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1794924-logger-assert/+merge/355823 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands
Continuous integration builds have changed state: Travis build 4067. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/434562141. Appveyor build 3863. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1794924_logger_assert-3863. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1794924-logger-assert/+merge/355823 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands.
Commit message:
Use std::cout for logging when calling terminate on shutdown, because the
logger might not exist any more.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1794924 in widelands: "Assertion "logger != nullptr failed on shutdown"
https://bugs.launchpad.net/widelands/+bug/1794924
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1794924-logger-assert/+merge/355823
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1794924-logger-assert into lp:widelands.
=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc 2018-09-24 08:35:06 +
+++ src/wlapplication.cc 2018-09-28 09:31:40 +
@@ -103,8 +103,9 @@
*/
#ifndef _WIN32
void terminate(int) {
- log("Waited 5 seconds to close audio. There are some problems here, so killing Widelands."
- " Update your sound driver and/or SDL to fix this problem\n");
+ // The logger can already be shut down, so we use cout
+ std::cout << "Waited 5 seconds to close audio. There are some problems here, so killing Widelands."
+ " Update your sound driver and/or SDL to fix this problem\n";
raise(SIGKILL);
}
#endif
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
