scripted-diff: Remove trailing newlines from one-line log messages #36397

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2610-log-format-string-trailing-newline changing 72 files +602 −602

maflcko commented at 11:58 AM on October 1, 2026: member
This is a low-prio style cleanup for years, but:
- It is tedious and possibly distracting to do it manually when touching the line for other reasons.
- The topic recently came up again when discussing log injection (https://github.com/bitcoin/bitcoin/pull/35833#issuecomment-5113804332)
So I wonder if a simple scripted-diff can cover the bulk of the changes now, with hopefully little conflicts with reviewed pull requests.

test: [move-only] Move test_LogEscapeMessage to logging tests

It is the only logging test in util tests and should sit with the other
logging tests instead.

Can be reviewed with --color-moved=dimmed-zebra

fa62e2e47b

scripted-diff: Remove trailing newlines from one-line log messages

The replacement skips log messages that contain: " or \
This keeps the script simple and allows to handle the special cases
manually.

-BEGIN VERIFY SCRIPT-

 sed --in-place --regexp-extended 's/((Log(Debug|Trace|Info|Warning|Error)|WalletLogPrintf)\([^\"]*"[^\"\\]*)\\n"([,)])/\1"\4/g' $(
       git grep -l --extended-regexp '(Log(Debug|Trace|Info|Warning|Error)|WalletLogPrintf)' ./src
 )

-END VERIFY SCRIPT-

fa791da6ab

DrahtBot renamed this:
~~scripted-diff: Remove trailing newlines from one-line log messages~~
scripted-diff: Remove trailing newlines from one-line log messages
on Oct 1, 2026
DrahtBot added the label Refactoring on Oct 1, 2026
DrahtBot commented at 11:58 AM on October 1, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36397.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #bitcoin-core/gui/939 (gui: show wallet name in progress messages by ShauryaaSharma)
- #36326 (kernel: Use typed errors for fatal and flush error notifications by arejula27)
- #36268 (refactor: prune unused semi-colons by fanquake)
- #36031 (wallet: Remove mapMasterKeys and enforce that only one encryption key can exist by achow101)
- #36015 (txorphanage: bound orphan memory by storing transactions serialized by brunoerg)
- #35998 (wallet: Handle or explicitly ignore WalletBatch write failures by achow101)
- #35906 (First steps towards a stateless, side-effect free validation library by purpleKarrot)
- #35902 (zmq: Log bind error at Error level, abort startup on init error by chriszeng1010)
- #35763 (util: write JSON atomically in WriteJson by kevkevinpal)
- #35733 (sign: Remove FillableSigningProvider by achow101)
- #35731 (Indexes: Harden the flush-error notification invariant by arejula27)
- #35724 (cmpctblock: Improve logging of cmpctblock message reconstruction statistics [part of prefill series] by davidgumberg)
- #35714 (validation: stop writes after flush failure by l0rinc)
- #35646 (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)
- #35570 (refactor: Change some validation.cpp methods to return BlockValidationState by optout21)
- #35569 (Encapsulation for CTransaction by purpleKarrot)
- #35558 (p2p: Prefill compact blocks by davidgumberg)
- #35502 (refactor: extract per-message helpers from ProcessMessage (move-only) by w0xlt)
- #35444 (wallet: make descriptor SPKM mutex non-recursive by w0xlt)
- #35354 (net: wait for validation queue flush for missing compact filter for an already known block by randomlogin)
- #35307 (blockstorage: keep snapshot base in normal blockfile range by shuv-amp)
- #35229 (refactor: Use CBlockIndex parameters as reference by optout21)
- #34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
- #34778 (logging: rewrite macros to enforce restrictions at compile-time, improve efficiency and usability by ryanofsky)
- #34729 (Reduce log noise by ajtowns)
- #34400 (wallet: parallel fast rescan (approx 8x speed up with 8 threads) by Eunovo)
- #34213 (net: preserve anchors when network is disabled by brunoerg)
- #34132 (coins, dbwrapper: remove error catcher, make point-read failures fatal by l0rinc)
- #34038 (logging: replace -loglevel with -trace, expose trace logging via RPC by ajtowns)
- #33663 (net: Filter addrman during address selection via AddrPolicy to avoid underfill by waketraindev)
- #33324 (blocks: add resumable reobfuscation for existing block files by l0rinc)
- #31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
- #30951 (net: option to disallow v1 connection on ipv4 and ipv6 peers by stratospher)
- #29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)
- #29256 (log, refactor: Allow log macros to accept context arguments by ryanofsky)
- #26022 (Add util::ResultPtr class by ryanofsky)
- #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
- #25665 (refactor: Add util::Result failure types and ability to merge result values by ryanofsky)
- #24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
maflcko marked this as a draft on Oct 1, 2026
maflcko commented at 12:10 PM on October 1, 2026: member

39 conflicts right now. Not sure what number is acceptable, but the focus should probably be to not invalidate any current acks.
willcl-ark commented at 12:33 PM on October 1, 2026: member

39 conflicts right now. Not sure what number is acceptable, but the focus should probably be to not invalidate any current acks.

I've wondered this before, but am again wondering if there's a way to have this drahtbot comment also display how many current (full) ACKs are on each of these? IIRC previously I thought it may be a bit tricky/probably not worth it, but with LLMs maybe it might be possible to show?

I think 39 conflicts is probably a strong enough signal on it's own here, but when there's 5 or 6 it can be useful (but tedious) to click each one and actually see the state of it...

Contributors

Labels

Linked (view graph)

#35833 log: prevent user input from injecting fake log lines

scripted-diff: Remove trailing newlines from one-line log messages #36397

Code Coverage & Benchmarks

Reviews

Conflicts