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
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
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.
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
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
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
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
05f9770c1f doc: Clarify developer notes about constant naming (Russell Yanofsky)
Pull request description:
I'm pretty sure developer notes were intended to say constants should be upper case and variables should be lower case, but right now they are ambiguous about whether to write:
```c++
extern const int SYMBOL;
```
or:
```c++
extern const int g_symbol;
```
First convention above is better than the second convention because it tells you without having to look anything up that the value of `SYMBOL` won't change at runtime. Also I haven't seen other c++ projects using the second convention.
ACKs for top commit:
MarcoFalke:
cr ACK 05f9770c1f
practicalswift:
ACK 05f9770c1f
jarolrod:
ACK 05f9770c1f🥃
Tree-SHA512: 766d0e25d9db818d45df4ad6386987014f2053584cbced4b755ceef8bda6b7e2cfeb34eb8516423bd03b140faaf577614d5e3be2799f7eed0eb439187ab85323
Since the flag has been removed from the RPC command table in commit
ec6902d0ea (PR #11179), this guideline
is not relevant anymore and can be removed.
a4a3fc4cd2 doc: improve subtree check instructions (Sjors Provoost)
Pull request description:
Running `git-subtree-check.sh` requires adding the subtree repository as a remote. I learned that several years ago and then forgot again.
This PR also improves the error message if the subtree commit can't be found.
ACKs for top commit:
laanwj:
ACK a4a3fc4cd2
fanquake:
ACK a4a3fc4cd2 - this looks ok.
Tree-SHA512: 959bd923726c172d17f9f97f8a56988bf2df5a94d3131e5152a66150b941394cee9e82fdc6b86e09c0ba91d123a496599f07ca454212168d8d301738394c12c8
