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 have neither reviewed nor tested the code changes.
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.
