diff options
| author | Ava Chow <github@achow101.com> | 2024-10-02 19:05:34 -0400 |
|---|---|---|
| committer | Ava Chow <github@achow101.com> | 2024-10-02 19:05:34 -0400 |
| commit | dda2613239b09e844717a06f255730c5f4c4aa4c (patch) | |
| tree | 930ae350b5d925b2eacfb7a1c1bfa64b6b8dae62 /test | |
| parent | e0ae9c14c4e39807ed27a8dff4680070d03f5d8d (diff) | |
| parent | fa2b7d8d6b3f8d53199921e1e542072441b26fab (diff) | |
Merge bitcoin/bitcoin#30929: log: Enforce trailing newline
fa2b7d8d6b3f8d53199921e1e542072441b26fab Remove redundant unterminated-logprintf tidy check (MarcoFalke)
bbbb2e43ee95c9a8866aa1f65e3f001f752dfed2 log: Enforce trailing newline, Remove redundant m_started_new_line (MarcoFalke)
Pull request description:
There are many problems around missing a trailing newline while logging:
* All log lines are currently terminated by a trailing newline. This means any runtime code trying to handle a "missing" newline is currently dead code.
* Leaving a line unterminated is racy and can cause content corruption by mixing log lines from different sources.
* It requires extra code like `m_started_new_line` to keep track of, which is annoying and pointless to maintain, because it is currently dead code, see https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1684380835.
* It requires a standalone `unterminated-logprintf` clang-tidy plugin, which is unmaintained (no one updated it for the new log function names), probably harder to maintain than normal C++ code (because it requires clang AST matcher knowledge), brittle (it can fail to detect issues at any time, if it goes out-of-sync, or be explicitly disabled via `NOLINT`), and annoying for devs (it is slow and intricate to run locally and thus only effectively run on CI or via the CI scripts).
Fix all issues by enforcing the trailing newline in logs directly in the code. Then remove all the other stuff.
This refactor does not change behavior.
ACKs for top commit:
stickies-v:
re-ACK fa2b7d8d6b3f8d53199921e1e542072441b26fab
achow101:
ACK fa2b7d8d6b3f8d53199921e1e542072441b26fab
ryanofsky:
Code review ACK fa2b7d8d6b3f8d53199921e1e542072441b26fab. Just comment and test cleanup since last review
Tree-SHA512: 10ed420f6c2fdb0f491d6c880be8dd2e8beef628f510adebadf4c3849d9f5e28906519d5cbaeb295f4c7c1b07c4c88a9905b3cfe30fee3a2c91ac9fd24ae6755
Diffstat (limited to 'test')
| -rwxr-xr-x | test/lint/lint-format-strings.py | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index a809851ec6..86a17fb0f8 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -62,7 +62,7 @@ def main(): matching_files_filtered = [] for matching_file in matching_files: - if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)|contrib/devtools/bitcoin-tidy/example_logprintf.cpp', matching_file): + if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)', matching_file): matching_files_filtered.append(matching_file) matching_files_filtered.sort() |