log: Remove NOLINT(bitcoin-unterminated-logprintf) #30485
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
| @@ -100,7 +100,7 @@ class CBitcoinLevelDBLogger : public leveldb::Logger { | |||
|
|
|||
| assert(p <= limit); | |||
| base[std::min(bufsize - 1, (int)(p - base))] = '\0'; | |||
| LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); // NOLINT(bitcoin-unterminated-logprintf) | |||
| LogDebug(BCLog::LEVELDB, "%s\n", util::RemoveSuffixView(base, "\n")); | |||
There was a problem hiding this comment.
Arguably, this is a "bugfix" to add a missing \n in the unlikely case where the buffer is exactly filled and the last character is overwritten from \n to \0.
However, I am not sure if anyone ever ran into this logging bug, so I am just leaving a comment here.
There was a problem hiding this comment.
(It is possible to test this "bugfix" by reducing both buffer sizes sufficiently and then running with -debug=leveldb -printtoconsole)
Github-Pull: bitcoin#30485 Rebased-From: fa18fc7
NOLINT(bitcoin-unterminated-logprintf)is used to document a missing trailing\nchar in the format string. This has many issues:\nends up in the formatted string. It is not enforced at compile-time, so it is brittle.NOLINT(bitcoin-unterminated-logprintf)were used to document a "continued" line, the log stream would be racy/corrupt, because any other thread may inject a log message in the meantime.m_started_new_line). This is problematic, because the presumed dead code has to be maintained (Early logging improvements #30386 (comment)).Fix almost all issues by removing the
NOLINT(bitcoin-unterminated-logprintf), ensuring that a new line is always present.A follow-up will remove the dead logging code.