From 3bf5f52808a5f7cd3958c9eea273d99fd11417ca Mon Sep 17 00:00:00 2001 From: Michael Ford Date: Sat, 13 Dec 2014 12:35:39 +0800 Subject: Create developer-notes.md Moves coding guidelines and development tips/tricks into a single file. Also adds a section explaining pull request terminology. --- doc/developer-notes.md | 186 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 doc/developer-notes.md (limited to 'doc/developer-notes.md') diff --git a/doc/developer-notes.md b/doc/developer-notes.md new file mode 100644 index 0000000000..eaeb90da1d --- /dev/null +++ b/doc/developer-notes.md @@ -0,0 +1,186 @@ +Coding +==================== + +Various coding styles have been used during the history of the codebase, +and the result is not very consistent. However, we're now trying to converge to +a single style, so please use it in new code. Old code will be converted +gradually. +- Basic rules specified in src/.clang-format. Use a recent clang-format-3.5 to format automatically. + - Braces on new lines for namespaces, classes, functions, methods. + - Braces on the same line for everything else. + - 4 space indentation (no tabs) for every block except namespaces. + - No indentation for public/protected/private or for namespaces. + - No extra spaces inside parenthesis; don't do ( this ) + - No space after function names; one space after if, for and while. + +Block style example: +```c++ +namespace foo +{ +class Class +{ + bool Function(char* psz, int n) + { + // Comment summarising what this section of code does + for (int i = 0; i < n; i++) { + // When something fails, return early + if (!Something()) + return false; + ... + } + + // Success return is usually at the end + return true; + } +} +} +``` + +Doxygen comments +----------------- + +To facilitate the generation of documentation, use doxygen-compatible comment blocks for functions, methods and fields. + +For example, to describe a function use: +```c++ +/** + * ... text ... + * @param[in] arg1 A description + * @param[in] arg2 Another argument description + * @pre Precondition for function... + */ +bool function(int arg1, const char *arg2) +``` +A complete list of `@xxx` commands can be found at http://www.stack.nl/~dimitri/doxygen/manual/commands.html. +As Doxygen recognizes the comments by the delimiters (`/**` and `*/` in this case), you don't +*need* to provide any commands for a comment to be valid, just a description text is fine. + +To describe a class use the same construct above the class definition: +```c++ +/** + * Alerts are for notifying old versions if they become too obsolete and + * need to upgrade. The message is displayed in the status bar. + * @see GetWarnings() + */ +class CAlert +{ +``` + +To describe a member or variable use: +```c++ +int var; //!< Detailed description after the member +``` + +Also OK: +```c++ +/// +/// ... text ... +/// +bool function2(int arg1, const char *arg2) +``` + +Not OK (used plenty in the current source, but not picked up): +```c++ +// +// ... text ... +// +``` + +A full list of comment syntaxes picked up by doxygen can be found at http://www.stack.nl/~dimitri/doxygen/manual/docblocks.html, +but if possible use one of the above styles. + +Development tips and tricks +--------------------------- + +**compiling for debugging** + +Run configure with the --enable-debug option, then make. Or run configure with +CXXFLAGS="-g -ggdb -O0" or whatever debug flags you need. + +**debug.log** + +If the code is behaving strangely, take a look in the debug.log file in the data directory; +error and debugging messages are written there. + +The -debug=... command-line option controls debugging; running with just -debug will turn +on all categories (and give you a very large debug.log file). + +The Qt code routes qDebug() output to debug.log under category "qt": run with -debug=qt +to see it. + +**testnet and regtest modes** + +Run with the -testnet option to run with "play bitcoins" on the test network, if you +are testing multi-machine code that needs to operate across the internet. + +If you are testing something that can run on one machine, run with the -regtest option. +In regression test mode, blocks can be created on-demand; see qa/rpc-tests/ for tests +that run in -regtest mode. + +**DEBUG_LOCKORDER** + +Bitcoin Core is a multithreaded application, and deadlocks or other multithreading bugs +can be very difficult to track down. Compiling with -DDEBUG_LOCKORDER (configure +CXXFLAGS="-DDEBUG_LOCKORDER -g") inserts run-time checks to keep track of which locks +are held, and adds warnings to the debug.log file if inconsistencies are detected. + +Locking/mutex usage notes +------------------------- + +The code is multi-threaded, and uses mutexes and the +LOCK/TRY_LOCK macros to protect data structures. + +Deadlocks due to inconsistent lock ordering (thread 1 locks cs_main +and then cs_wallet, while thread 2 locks them in the opposite order: +result, deadlock as each waits for the other to release its lock) are +a problem. Compile with -DDEBUG_LOCKORDER to get lock order +inconsistencies reported in the debug.log file. + +Re-architecting the core code so there are better-defined interfaces +between the various components is a goal, with any necessary locking +done by the components (e.g. see the self-contained CKeyStore class +and its cs_KeyStore lock for example). + +Threads +------- + +- ThreadScriptCheck : Verifies block scripts. + +- ThreadImport : Loads blocks from blk*.dat files or bootstrap.dat. + +- StartNode : Starts other threads. + +- ThreadDNSAddressSeed : Loads addresses of peers from the DNS. + +- ThreadMapPort : Universal plug-and-play startup/shutdown + +- ThreadSocketHandler : Sends/Receives data from peers on port 8333. + +- ThreadOpenAddedConnections : Opens network connections to added nodes. + +- ThreadOpenConnections : Initiates new connections to peers. + +- ThreadMessageHandler : Higher-level message handling (sending and receiving). + +- DumpAddresses : Dumps IP addresses of nodes to peers.dat. + +- ThreadFlushWalletDB : Close the wallet.dat file if it hasn't been used in 500ms. + +- ThreadRPCServer : Remote procedure call handler, listens on port 8332 for connections and services them. + +- BitcoinMiner : Generates bitcoins (if wallet is enabled). + +- Shutdown : Does an orderly shutdown of everything. + +Pull Request Terminology +------------------------ + +Concept ACK - Agree with the idea and overall direction, but haven't reviewed the code changes or tested them. + +utACK (untested ACK) - Reviewed and agree with the code changes but haven't actually tested them. + +Tested ACK - Reviewed the code changes and have verified the functionality or bug fix. + +ACK - A loose ACK can be confusing. It's best to avoid them unless it's a documentation/comment only change in which case there is nothing to test/verify; therefore the tested/untested distinction is not there. + +NACK - Disagree with the code changes/concept. Should be accompanied by an explanation. -- cgit v1.2.3