Commit graph

205 commits

Author SHA1 Message Date
MarcoFalke
fa780e1c25
build: Remove --enable-gprof
This reverts cfaac2a60f
2024-06-09 22:45:29 +02:00
fanquake
cbd4640ede
build: remove --enable-lcov-branch-coverage
This supports lcov 2.x in the sense that we are no-longer hardcoding
version specific options, and instead will use the `LCOV_OPTS` variable
(which is the more correct/flexible thing to do in any case). It's also
quite likely that devs are already having to pass extra options to lcov
2.x, given it's more stringent in terms of coverage generation and error
checking. See this thread for an example:
https://github.com/linux-test-project/lcov/issues/238.

Added an example to the developer notes.

Tested on one machine (LCOV 2.0, gcc 13.2) with:
```bash
./autogen.sh
./configure --enable-lcov CXXFLAGS="-fprofile-update=prefer-atomic" LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch"
make
make cov
<snip>
Processing file src/netaddress.cpp
  lines=521 hit=480 functions=72 hit=72 branches=675 hit=499
Overall coverage rate:
  lines......: 81.8% (79362 of 97002 lines)
  functions......: 77.8% (10356 of 13310 functions)
  branches......: 49.6% (130628 of 263196 branches)
```

and another machine (LCOV 2.1, Clang 18.1.3) with:
```bash
./autogen.sh
./configure --enable-lcov CC=clang CXX=clang++ LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch,inconsistent"
make
make cov
<snip>
Processing file src/util/strencodings.cpp
  lines=315 hit=311 functions=38 hit=38 branches=425 hit=357
Overall coverage rate:
  source files: 622
  lines.......: 79.8% (70311 of 88132 lines)
  functions...: 78.1% (13968 of 17881 functions)
  branches....: 44.5% (157551 of 354317 branches)
Message summary:
  101 warning messages:
    count: 1
    inconsistent: 100
  3528 ignore messages:
    inconsistent: 3528
```
2024-05-30 10:36:55 +01:00
willcl-ark
4b9f49da2b
doc: fix broken relative md links
These relative links in our documentation are broken, fix them.
2024-05-03 16:07:12 +01:00
dergoegge
78407b99ed [clang-tidy] Enable the misc-no-recursion check
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Gloria Zhao <gloriajzhao@gmail.com>
2024-04-07 14:04:45 +01:00
spicyzboss
4f1753deaa
doc: Wrap flags with code in developer-notes.md 2024-03-08 15:55:37 +07:00
Cory Fields
f8a06f7a02 doc: remove references to disable-asm option now that it's gone
The comment about sha256_sse4::Transform is believed to be old and stale.
2024-02-29 19:10:31 +00:00
Andrew Toth
eb5d78c649
doc: document preference for list-initialization 2024-02-23 13:19:19 -05:00
Fabian Jahr
c003562120
doc: Add missing backtick in developer notes logging section
Also fix some minor punctuation error in the section.
2024-01-12 16:26:17 +01:00
Ava Chow
7ff8e6b240
Merge bitcoin/bitcoin#28318: logging: Simplify API for level based logging
e60fc7d5d3 logging: Replace uses of LogPrintfCategory (Anthony Towns)
f7ce5ac08c logging: add LogError, LogWarning, LogInfo, LogDebug, LogTrace (Anthony Towns)
fbd7642c8e logging: add -loglevelalways=1 option (Anthony Towns)
782bb6a056 logging: treat BCLog::ALL like BCLog::NONE (Anthony Towns)
667ce3e329 logging: Drop BCLog::Level::None (Anthony Towns)
ab34dc6012 logging: Log Info messages unconditionally (Anthony Towns)
dfe98b6874 logging: make [cat:debug] and [info] implicit (Anthony Towns)
c5c76dc615 logging: refactor: pull prefix code out (Anthony Towns)

Pull request description:

  Replace `LogPrint*` functions with severity based logging functions:

   * `LogInfo(...)`, `LogWarning(...)`, `LogError(...)` for unconditional (uncategorised) logging (replaces `LogPrintf`)
   * `LogDebug(CATEGORY, ...)` and `LogTrace(CATEGORY, ...)` for conditional logging (replaces `LogPrint`)
   * `LogPrintLevel(CATEGORY, LEVEL, ...)` for when the level isn't known in advance, or a category needs to be added for an info/warning/error log message (mostly unchanged, but rarely needed)

  Logs look roughly as they do now with `LogInfo` not having an `[info]` prefix, and `LogDebug` having a `[cat]` prefix, rather than a `[cat:debug]` prefix. This removes `BCLog::Level::None` entirely -- for `LogFlags::NONE` just use `Level::Info`, for any actual category, use `Level::Debug`.

  Adds docs to developer-notes about when to use which level.

  Adds `-loglevelalways=1` option so that you get `[net:debug]`, `[all:info]`, `[all:warning]` etc, which might be helpful for automated parsing, or just if you like everything to be consistent. Defaults to off to reduce noise in the default config, and to avoid unnecessary changes on upgrades.

  Changes the behaviour of `LogPrintLevel(CATEGORY, BCLog::Level::Info, ...)` to be logged unconditionally, rather than only being an additional optional logging level in addition to trace and debug. Does not change the behaviour of `LogPrintLevel(NONE, Debug, ...)` and `LogPrintLevel(NONE, Trace, ...)` being no-ops.

ACKs for top commit:
  maflcko:
    re-ACK e60fc7d5d3 🌚
  achow101:
    ACK e60fc7d5d3
  stickies-v:
    ACK e60fc7d5d3
  jamesob:
    ACK e60fc7d5d3 ([`jamesob/ackr/28318.1.ajtowns.logging_simplify_api_for`](https://github.com/jamesob/bitcoin/tree/ackr/28318.1.ajtowns.logging_simplify_api_for))

Tree-SHA512: e7a4588779b148242495b7b6f64198a00c314cd57100affab11c43e9d39c9bbf85118ee2002792087fdcffdea08c84576e20844b3079f27083e26ddd7ca15d7f
2024-01-10 14:11:32 -05:00
Anthony Towns
f7ce5ac08c logging: add LogError, LogWarning, LogInfo, LogDebug, LogTrace
These provide simple and clear ways to write the most common logging
operations:

    LogInfo("msg");
    LogDebug(BCLog::LogFlags::NET, "msg");

    LogError("msg");
    LogWarning("msg");
    LogTrace(BCLog::LogFlags::NET, "msg");

For cases where the level cannot be hardcoded, LogPrintLevel(category,
level, ...) remains available.
2023-12-20 15:59:48 +10:00
MarcoFalke
6666713041
refactor: Rename fs::path::u8string() to fs::path::utf8string() 2023-12-14 16:22:40 +01:00
Andrew Chow
e789b30b25
Merge bitcoin/bitcoin#27116: doc: clarify that LOCK() internally checks whether the mutex is held
91d0888921 sync: unpublish LocksHeld() which is used only in sync.cpp (Vasil Dimov)
3df37e0c78 doc: clarify that LOCK() does AssertLockNotHeld() internally (Vasil Dimov)

Pull request description:

  Constructs like

  ```cpp
  AssertLockNotHeld(m);
  LOCK(m);
  ```

  are equivalent to (almost, modulo some logging differences, see below)

  ```cpp
  LOCK(m);
  ```

  for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case.

  Also remove `LocksHeld()` from the public interface in `sync.h` since it is only used in `sync.cpp`.

ACKs for top commit:
  achow101:
    ACK 91d0888921
  hebasto:
    ACK 91d0888921, I have reviewed the code and it looks OK.

Tree-SHA512: c4b7ef2c0bfeb28d1c4f55f497810f629873137e02f5a92137c02cb1ff603ac76473dcd2171e594491494a5cb87b8c0c803e06b86f190d4acb231791e28e802d
2023-10-26 15:02:13 -04:00
Greg Sanders
2ce7e31d2d docs: Add reference to total.coverage report 2023-10-18 09:27:50 -04:00
MarcoFalke
fa6e6a3f03
doc: Remove confusing assert linter 2023-09-14 18:59:52 +02:00
Martin Zumsande
2ebc7e68cc
doc: describe 'init load' thread actions 2023-07-07 19:31:27 -03:00
furszy
04575106b2
scripted-diff: rename 'loadblk' thread name to 'initload'
The thread does not only load blocks, it loads the mempool and,
in a future commit, will start the indexes as well.

Also, renamed the 'ThreadImport' function to 'ImportBlocks'
And the 'm_load_block' class member to 'm_thread_load'.

-BEGIN VERIFY SCRIPT-

sed -i "s/ThreadImport/ImportBlocks/g" $(git grep -l ThreadImport -- ':!/doc/')
sed -i "s/loadblk/initload/g" $(git grep -l loadblk -- ':!/doc/release-notes/')
sed -i "s/m_load_block/m_thread_load/g" $(git grep -l m_load_block)

-END VERIFY SCRIPT-
2023-07-07 19:31:27 -03:00
fanquake
3b2acfcfec
build: suppress external warnings by default 2023-06-15 14:12:10 +01:00
Riahiamirreza
f24f4fa3f1
Update developer-notes.md 2023-04-14 20:11:51 +03:30
fanquake
3d53a85ddf
Merge bitcoin/bitcoin#27220: doc: update broken str util reference links on developer-notes
da347de530 doc: update broken links (pablomartin4btc)

Pull request description:

  References to `utilstrencodings` and `lint-locale-dependence.sh` where incorrect, updating them accordingly.

  Also, adding another reference to util function [`LocaleIndependentAtoi`](https://github.com/bitcoin/bitcoin/blob/master/src/util/strencodings.h#L108-L118), which is related with the updated section of the guide:

  ```
  // LocaleIndependentAtoi is provided for backwards compatibility reasons.
  //
  // New code should use ToIntegral or the ParseInt* functions
  // which provide parse error feedback.
  //
  // The goal of LocaleIndependentAtoi is to replicate the defined behaviour of
  // std::atoi as it behaves under the "C" locale, and remove some undefined
  // behavior. If the parsed value is bigger than the integer type's maximum
  // value, or smaller than the integer type's minimum value, std::atoi has
  // undefined behavior, while this function returns the maximum or minimum
  // values, respectively.
  ```

ACKs for top commit:
  MarcoFalke:
    lgtm ACK da347de530

Tree-SHA512: c8f4cd9cff1fb3ea367ac9dbe5aa45dc187fc60114f2e2106e02e0e17fea4ee34d6e0c408fe920c2d8765e06b4dc30c231f0454fa35469c4399e0cadbcd341ba
2023-03-11 11:02:03 +01:00
TheCharlatan
54c4d03578
doc: Show how less noisy clang-tidy output can be achieved 2023-03-10 16:47:41 +01:00
pablomartin4btc
da347de530 doc: update broken links 2023-03-08 18:26:41 -03:00
fanquake
84ca5b349e
doc: mention sanitizer suppressions in developer docs
Should be enough to close #17834.
2023-02-24 12:16:37 +00:00
Vasil Dimov
3df37e0c78
doc: clarify that LOCK() does AssertLockNotHeld() internally
Constructs like

```cpp
AssertLockNotHeld(m);
LOCK(m);
```

are equivalent to

```cpp
LOCK(m);
```

for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in
such cases.
2023-02-17 11:42:40 +01:00
fanquake
8d69b614cc
Merge bitcoin/bitcoin#23810: docs: avoid C-style casts; use modern C++ casts
75347236f2 docs: document c-style cast prohibition (Pasta)

Pull request description:

  In the words of practicalswift:
  ```
  A C-style cast is equivalent to try casting in the following order:

      const_cast(...)
      static_cast(...)
      const_cast(static_cast(...))
      reinterpret_cast(...)
      const_cast(reinterpret_cast(...))

  By using static_cast<T>(...) explicitly we avoid the possibility of an unintentional and
  dangerous reinterpret_cast. Furthermore static_cast<T>(...) allows for easier grepping of casts.

  For a more thorough discussion, see "ES.49: If you must use a cast, use a named cast"
  in the C++ Core Guidelines (Stroustrup & Sutter).
  ```

  Modern tooling, specifically `-Wold-style-cast` can enable us to enforce never using C-style casts. I believe this is especially important due to the number of C-style casts the codebase is currently being used as a reinterpret_cast. reinterpret_casts are especially dangerous, and should never be done via C-style casts.

  Update the docs to suggest the use of named cast or functional casts.

Top commit has no ACKs.

Tree-SHA512: 29a98de396f0c78e32d8a1831319162203c4405a670da5add5da956fcc7df200a1cec162ef1cfac4ddfb02714b66406081d40ed435c7f0f28581cfa24d94fac1
2023-02-08 10:36:43 +00:00
James O'Beirne
00eeb31c76 scripted-diff: rename CChainState -> Chainstate
-BEGIN VERIFY SCRIPT-
sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*')
-END VERIFY SCRIPT-

Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-09-09 11:47:27 -04:00
Jon Atack
9580480570 Update debug logging section in the developer notes 2022-08-20 13:40:03 +02:00
Vasil Dimov
dc02edcba1
doc: update the URLs to thread functions in developer-notes
ThreadMapPort() does not appear on doxygen.bitcoincore.org
because it is inside `#ifdef`.
2022-07-11 09:56:24 +02:00
Vasil Dimov
c5cc3f140c
doc: list the I2P accept thread in developer-notes 2022-07-07 15:07:07 +02:00
fanquake
d873ff96e5
refactor: cleanups post unsubtree'ing univalue
Mostly changes to remove src/univalue exceptions from the various linters,
and the required code changes to make them happy. As well as minor doc
changes.
2022-06-15 12:56:44 +01:00
MacroFake
8f3ab9a1b1
Merge bitcoin/bitcoin#24931: Strengthen thread safety assertions
ce893c0497 doc: Update developer notes (Anthony Towns)
d2852917ee sync.h: Imply negative assertions when calling LOCK (Anthony Towns)
bba87c0553 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns)
a559509a0b sync.h: Add GlobalMutex type (Anthony Towns)
be6aa72f9f qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns)
f24bd45b37 net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns)

Pull request description:

  This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions.

  This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex`  to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`).

ACKs for top commit:
  MarcoFalke:
    review ACK ce893c0497 🐦
  hebasto:
    ACK ce893c0497

Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
2022-06-10 16:42:53 +02:00
Jon Atack
433b525694 Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes
that was added in 2015 by commit b8c06ef40 in PR 7003, as that potential issue
would now be caught by the test/lint/lint-format-strings.py script run by the CI
2022-06-07 15:56:26 +02:00
laanwj
dd52f79a73
Merge bitcoin/bitcoin#24416: doc: Avoid ADL for function calls
52a797bfe5 doc: Avoid ADL for function calls (Hennadii Stepanov)

Pull request description:

  It happened two times recently, when [ADL](https://en.cppreference.com/w/cpp/language/adl) popped up unexpectedly and brought some confusion:
  - https://github.com/bitcoin/bitcoin/pull/24338/files#r805989994
  > Any idea why this even compiles?
  - https://www.erisian.com.au/bitcoin-core-dev/log-2022-02-18.html#l-51:
  > 2022-02-18T03:24:14  \<dongcarl\> Does anyone know why this compiles? 6d3d2caa37
  > 2022-02-18T03:24:14  \<dongcarl\> GetUTXOStatsWithHasher and MakeUTXOHasher are both in the `kernel::` namespace and I never added a `using` declaration on top...
  > 2022-02-18T03:25:53  \<sipa\> https://en.cppreference.com/w/cpp/language/adl ?

  Let's document our intention to avoid similar cases in the future.

ACKs for top commit:
  laanwj:
    Anyhow, ACK 52a797bfe5, there is no need to hold merge up on this, documenting it is a step forward.

Tree-SHA512: f52688b5d8f6130302185206ec6ea4731b099a75294ea2d477901a52d6d58473e3427e658aea408c140c2824c37a0399ec7376aded2a91197895ea52d51f0018
2022-06-01 13:32:48 +02:00
Anthony Towns
ce893c0497 doc: Update developer notes 2022-05-21 01:23:23 +10:00
Jon Atack
bd5dbc30db doc: update developer notes wrt --enable-debug and DEBUG_LOCKCONTENTION 2022-05-20 12:48:17 +02:00
Jon Atack
654284209f Add clang lifetimebound section to developer notes 2022-05-10 16:29:26 +02:00
Jon Atack
e66b321fd1 Add C++ functions and methods section to developer notes
Credit for some parts to the Google C++ Style Guide "Inputs and Outputs"
section at https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs

Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>
2022-05-10 14:57:50 +02:00
Jon Atack
5fca70f5b1 Link in developer notes style to internal interface exception 2022-05-09 16:14:04 +02:00
Jon Atack
fc4cb857cc Prefer Python for scripts in developer notes
along with a few miscellaneous touch-ups.
2022-05-09 16:13:38 +02:00
Jon Atack
370120ec2f Remove obsolete BDB ENABLE_WALLET section in developer notes 2022-05-06 18:29:59 +02:00
Jon Atack
8b3f1e30f0
Update RPC argument and field naming guideline in developer notes
Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>
2022-04-08 15:46:05 +02:00
Jon Atack
4394733331
Add DEBUG_LOCKCONTENTION documentation to the developer notes 2022-04-05 12:49:50 +02:00
MarcoFalke
67f654ef61
doc: Document clang-tidy in dev notes 2022-03-25 08:18:51 +00:00
Hennadii Stepanov
52a797bfe5
doc: Avoid ADL for function calls 2022-02-22 15:25:51 +02:00
Jon Atack
48742693ac
Replace "can not" with "cannot" in docs, user messages, and tests 2022-02-21 19:07:29 +01:00
Pasta
75347236f2 docs: document c-style cast prohibition 2022-02-02 00:31:47 +07:00
Russell Yanofsky
9b575f1c73 Improve fs::PathToString documentation 2021-11-15 12:08:49 -05:00
Gleb Naumenko
0659f12b13
Add minisketch dependency 2021-10-21 09:38:55 +08:00
fanquake
5b7210c874
Merge bitcoin/bitcoin#22409: configure: keep relative paths in debug info
0bc666b053 doc: add info for debugging with relative paths (S3RK)
a8b515c317 configure: keep relative paths in debug info (S3RK)

Pull request description:

  This is a follow-up for #20353 that fixes #21885

  It also adds a small section to assist debugging without absolute paths in debug info.

ACKs for top commit:
  kallewoof:
    Tested ACK 0bc666b053
  Zero-1729:
    Light crACK 0bc666b053

Tree-SHA512: d4b75183c3d3a0f59fe786841fb230581de87f6fe04cf7224e4b89c520d45513ba729d4ad8c0e62dd1dbaaa7a25741f04d036bc047f92842e76c9cc31ea47fb2
2021-10-11 10:34:29 +08:00
fanquake
2b90eae33c
doc: update developer docs for subtree renaming 2021-09-29 15:12:12 +08:00
Jeremy Rand
9bdda50151
Enable TLS in links in documentation 2021-09-16 22:00:20 +00:00