diff --git a/arcanist/linter/CppVoidParameterLinter.php b/arcanist/linter/CppVoidParameterLinter.php --- a/arcanist/linter/CppVoidParameterLinter.php +++ b/arcanist/linter/CppVoidParameterLinter.php @@ -53,7 +53,7 @@ $absPath = Filesystem::resolvePath($path, $this->getProjectRoot()); $fileContent = Filesystem::readFile($absPath); - if (preg_match_all('/[^\s{]+\s?\(void\)/', $fileContent, $voidParameters, + if (preg_match_all('/[^\s{(]+\s?\(void\)/', $fileContent, $voidParameters, PREG_OFFSET_CAPTURE)) { foreach ($voidParameters[0] as $voidParameter) { list($function, $offset) = $voidParameter; diff --git a/doc/developer-notes.md b/doc/developer-notes.md --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -276,6 +276,33 @@ compiler flags. This inserts run-time checks to keep track of which locks are held, and adds warnings to the debug.log file if inconsistencies are detected. +### Assertions and Checks + +The util file `src/util/check.h` offers helpers to protect against coding and +internal logic bugs. They must never be used to validate user, network or any +other input. + +* `assert` or `Assert` should be used to document assumptions when any + violation would mean that it is not safe to continue program execution. The + code is always compiled with assertions enabled. + - For example, a nullptr dereference or any other logic bug in validation + code means the program code is faulty and must terminate immediately. +* `CHECK_NONFATAL` should be used for recoverable internal logic bugs. On + failure, it will throw an exception, which can be caught to recover from the + error. + - For example, a nullptr dereference or any other logic bug in RPC code + means that the RPC code is faulty and can not be executed. However, the + logic bug can be shown to the user and the program can continue to run. +* `Assume` should be used to document assumptions when program execution can + safely continue even if the assumption is violated. In debug builds it + behaves like `Assert`/`assert` to notify developers and testers about + nonfatal errors. In production it doesn't warn or log anything, though the + expression is always evaluated. + - For example it can be assumed that a variable is only initialized once, + but a failed assumption does not result in a fatal bug. A failed + assumption may or may not result in a slightly degraded user experience, + but it is safe to continue program execution. + ### Valgrind suppressions file Valgrind is a programming tool for memory debugging, memory leak detection, and diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -112,9 +112,8 @@ add_compile_options_to_configuration_for_language(Debug ${LANGUAGE} ${COMPILER_DEBUG_LEVEL}) endforeach() -# Define the debugging symbols DEBUG and DEBUG_LOCKORDER when the Debug build -# type is selected. -add_compile_definitions_to_configuration(Debug DEBUG DEBUG_LOCKORDER) +# Define some debugging symbols when the Debug build type is selected. +add_compile_definitions_to_configuration(Debug DEBUG DEBUG_LOCKORDER ABORT_ON_FAILED_ASSUME) # Add -ftrapv when building in Debug add_compile_options_to_configuration(Debug -ftrapv) diff --git a/src/util/check.h b/src/util/check.h --- a/src/util/check.h +++ b/src/util/check.h @@ -47,18 +47,33 @@ #error "Cannot compile without assertions!" #endif -/** Helper for Assert(). TODO remove in C++14 and replace - * `decltype(get_pure_r_value(val))` with `T` (templated lambda) */ +/** Helper for Assert() */ template T get_pure_r_value(T &&val) { return std::forward(val); } /** Identity function. Abort if the value compares equal to zero */ #define Assert(val) \ - [&]() -> decltype(get_pure_r_value(val)) { \ + ([&]() -> decltype(get_pure_r_value(val)) { \ auto &&check = (val); \ assert(#val &&check); \ return std::forward(check); \ - }() + }()) + +/** + * Assume is the identity function. + * + * - Should be used to run non-fatal checks. In debug builds it behaves like + * Assert()/assert() to notify developers and testers about non-fatal errors. + * In production it doesn't warn or log anything. + * - For fatal errors, use Assert(). + * - For non-fatal errors in interactive sessions (e.g. RPC or command line + * interfaces), CHECK_NONFATAL() might be more appropriate. + */ +#ifdef ABORT_ON_FAILED_ASSUME +#define Assume(val) Assert(val) +#else +#define Assume(val) ((void)(val)) +#endif #endif // BITCOIN_UTIL_CHECK_H