Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes #25292

pull jonatack wants to merge 1 commits into bitcoin:master from jonatack:update-lint-format-strings-and-dev-notes changing 2 files +1 −5

jonatack commented at 1:58 PM on June 7, 2022: member

added by #7003 in 2015, as that potential issue would now be caught by the test/lint/lint-format-strings.py script run by the CI.

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

433b525694

jonatack commented at 2:00 PM on June 7, 2022: member

Was wondering whether to update that section in the developer notes with LogPrintLevel and LogPrintfCategory, but it seems reasonable to just remove it now.
MarcoFalke commented at 2:05 PM on June 7, 2022: member

cr ACK 433b52569417674f84c2b1d449037701814420c4
w0xlt approved
w0xlt commented at 2:15 PM on June 7, 2022: contributor

ACK https://github.com/bitcoin/bitcoin/pull/25292/commits/433b52569417674f84c2b1d449037701814420c4
in doc/developer-notes.md:836 in 433b525694
```
 830 | @@ -831,11 +831,6 @@ int GetInt(Tabs tab)
 831 |  Strings and formatting
 832 |  ------------------------
 833 |  
 834 | -- Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not.
 835 | -
 836 | -  - *Rationale*: Confusion of these can result in runtime exceptions due to
```
MarcoFalke commented at 2:26 PM on June 7, 2022:

I don't even understand how this can lead to runtime errors. Trying to match an enum to a format string should never succeed at compile time.

cc @laanwj who added this in commit b8c06ef409792dd9a6d14d46b50787fa7a6fb33d.
laanwj commented at 7:12 PM on June 7, 2022:

Category was a string, not an enum back in 2015.

src/addrman.cpp: LogPrint("addrman", "Moving %s to tried\n", addr.ToString());

So it was possible to get it wrong. I agree this is not longer the case as it is type-safe now.
jonatack commented at 7:23 PM on June 7, 2022:

Thanks for the context!
MarcoFalke assigned laanwj on Jun 7, 2022
DrahtBot added the label Docs on Jun 7, 2022
DrahtBot added the label Tests on Jun 7, 2022
laanwj merged this on Jun 7, 2022
laanwj closed this on Jun 7, 2022
jonatack deleted the branch on Jun 7, 2022
sidhujag referenced this in commit dc52189a2c on Jun 8, 2022
DrahtBot locked this on Jun 7, 2023

Contributors

Labels

Linked (view graph)

#7003 doc: Add non-style-related development guidelines