Add attribute [[noreturn]] (C++11) to functions that will not return.
Rationale:
38@@ -39,7 +39,7 @@
39 #include <openssl/err.h>
40 #include <openssl/rand.h>
41
42-static void RandFailure()
43+[[noreturn]] static void RandFailure()
44 {
45 LogPrintf("Failed to read randomness, aborting\n");
46 abort();
@laanwj Yes, originally I aimed to add the [[noreturn]] attribute also to:
BitcoinApplication::handleRunawayException(const QString &message) (in qt/bitcoin.cpp)Shutdown()/StartShutdown() (in test/test_bitcoin_main.cpp)But I ran into some compilation issues that I didn’t have time to follow up on, so I excluded them from this PR.
Rationale:
* Reduce the number of false positives from static analyzers
* Potentially enable additional compiler optimizations
518@@ -519,7 +519,7 @@ void BitcoinApplication::shutdownResult()
519 quit(); // Exit main loop after shutdown finished
520 }
521
522-void BitcoinApplication::handleRunawayException(const QString &message)
523+[[noreturn]] void BitcoinApplication::handleRunawayException(const QString &message)
524 {
525 QMessageBox::critical(0, "Runaway exception", BitcoinGUI::tr("A fatal error occurred. Bitcoin can no longer continue safely and will quit.") + QString("\n\n") + message);
526 ::exit(EXIT_FAILURE);
I’ve now added the [[noreturn]] attribute to Shutdown()/StartShutdown() (in test/test_bitcoin_main.cpp).
However, after some digging it appears that [[noreturn]] might not be correct for BitcoinApplication::handleRunawayException(const QString &message). If I understand it correctly it calls QCoreApplication::exit(…) which does return to the caller (it is event processing that stops).
This PR should now be ready for review.
If I understand it correctly it calls QCoreApplication::exit(…) which does return to the caller (it is event processing that stops).
Agreed - if you’re not 100% sure that a function doesn’t return, it’s much better to err on the safe side and not specify it. From experience, functions with noreturn returning can result in some awful things, which might even be security critical (but at the least hard to debug), as it can fall through to apparently random code.
@TheBlueMatt You’re absolutely right! I was wrong about QCoreApplication::exit(…) being called. Sorry :-)
After applying 536504c8bef776902fa071dd76a37ef7037c1945 make check passes for all build configurations except HOST=x86_64-apple-darwin11, see https://travis-ci.org/bitcoin/bitcoin/builds/254651379. I don’t understand why that is.
@theuni Yes, I thought so but when applying d52e345a6e7526f516425b7547dbca534a884172 I got the parsing error qt/bitcoin.cpp:[…]: Parse error at ";" for all Qt-enabled builds, see https://travis-ci.org/bitcoin/bitcoin/jobs/254702626#L2290.
0$ make V=1
1[…]
2QT_SELECT=qt5 /usr/lib/x86_64-linux-gnu/qt5/bin/moc -I. -I../src/config -I/usr/include/x86_64-linux-gnu/qt5/QtNetwork -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtWidgets -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtGui -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtCore -I/usr/include/x86_64-linux-gnu/qt5 -DHAVE_CONFIG_H -I. qt/bitcoin.cpp | \
3 /bin/sed -e '/^\*\*.*Created:/d' -e '/^\*\*.*by:/d' > qt/bitcoin.moc
4qt/bitcoin.cpp:233: Parse error at ";"
5[…]
6$ wc -c src/qt/bitcoin.moc
70 src/qt/bitcoin.moc
8$ /usr/lib/x86_64-linux-gnu/qt5/bin/moc --version
9moc 5.5.1
Qt:s moc being confused by the [[noreturn]] attribute? :-)
I’ve now excluded the commit touching handleRunawayException(…) (src/qt/bitcoin.cpp). Qt’s moc is doing too much stuff behind the scenes for me to be comfortable doing that change :-)
This PR should now be ready for review.
