From 5c21e6c6d3843232f384079837da1d9fae573b8d Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 5 Apr 2018 10:17:32 -0400 Subject: [PATCH 1/2] [logging] Comment all continuing logs. Most logs should terminated with a '\n'. Some logs are built up over multiple calls to logPrintf(), so do not need a newline terminater. Comment all of these 'continued' logs as a linter hing. --- src/dbwrapper.cpp | 2 +- src/init.cpp | 2 +- src/sync.cpp | 8 ++++---- src/txdb.cpp | 4 ++-- src/validation.cpp | 8 ++++---- src/wallet/wallet.cpp | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index ca446c92f..e401b5fb1 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -63,7 +63,7 @@ public: assert(p <= limit); base[std::min(bufsize - 1, (int)(p - base))] = '\0'; - LogPrintf("leveldb: %s", base); + LogPrintf("leveldb: %s", base); /* Continued */ if (base != buffer) { delete[] base; } diff --git a/src/init.cpp b/src/init.cpp index 4bb2bc2c3..b1897e0c1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1215,7 +1215,7 @@ bool AppInitMain() // Warn about relative -datadir path. if (gArgs.IsArgSet("-datadir") && !fs::path(gArgs.GetArg("-datadir", "")).is_absolute()) { - LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " + LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " /* Continued */ "current working directory '%s'. This is fragile, because if bitcoin is started in the future " "from a different location, it will be unable to locate the current data files. There could " "also be data loss if bitcoin is started while in a temporary directory.\n", diff --git a/src/sync.cpp b/src/sync.cpp index fdf7c44e4..6f21d498e 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -81,20 +81,20 @@ static void potential_deadlock_detected(const std::pair& mismatch, LogPrintf("Previous lock order was:\n"); for (const std::pair & i : s2) { if (i.first == mismatch.first) { - LogPrintf(" (1)"); + LogPrintf(" (1)"); /* Continued */ } if (i.first == mismatch.second) { - LogPrintf(" (2)"); + LogPrintf(" (2)"); /* Continued */ } LogPrintf(" %s\n", i.second.ToString()); } LogPrintf("Current lock order is:\n"); for (const std::pair & i : s1) { if (i.first == mismatch.first) { - LogPrintf(" (1)"); + LogPrintf(" (1)"); /* Continued */ } if (i.first == mismatch.second) { - LogPrintf(" (2)"); + LogPrintf(" (2)"); /* Continued */ } LogPrintf(" %s\n", i.second.ToString()); } diff --git a/src/txdb.cpp b/src/txdb.cpp index 8550a7e88..45ce94ae4 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -370,7 +370,7 @@ bool CCoinsViewDB::Upgrade() { int64_t count = 0; LogPrintf("Upgrading utxo-set database...\n"); - LogPrintf("[0%%]..."); + LogPrintf("[0%%]..."); /* Continued */ uiInterface.ShowProgress(_("Upgrading UTXO database"), 0, true); size_t batch_size = 1 << 24; CDBBatch batch(db); @@ -389,7 +389,7 @@ bool CCoinsViewDB::Upgrade() { uiInterface.ShowProgress(_("Upgrading UTXO database"), percentageDone, true); if (reportDone < percentageDone/10) { // report max. every 10% step - LogPrintf("[%d%%]...", percentageDone); + LogPrintf("[%d%%]...", percentageDone); /* Continued */ reportDone = percentageDone/10; } } diff --git a/src/validation.cpp b/src/validation.cpp index df8729e38..26d2b0b7e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2231,13 +2231,13 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar DoWarning(strWarning); } } - LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)", __func__, + LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)", __func__, /* Continued */ pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion, log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx, FormatISO8601DateTime(pindexNew->GetBlockTime()), GuessVerificationProgress(chainParams.TxData(), pindexNew), pcoinsTip->DynamicMemoryUsage() * (1.0 / (1<<20)), pcoinsTip->GetCacheSize()); if (!warningMessages.empty()) - LogPrintf(" warning='%s'", boost::algorithm::join(warningMessages, ", ")); + LogPrintf(" warning='%s'", boost::algorithm::join(warningMessages, ", ")); /* Continued */ LogPrintf("\n"); } @@ -3903,14 +3903,14 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, int nGoodTransactions = 0; CValidationState state; int reportDone = 0; - LogPrintf("[0%%]..."); + LogPrintf("[0%%]..."); /* Continued */ for (CBlockIndex* pindex = chainActive.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { boost::this_thread::interruption_point(); int percentageDone = std::max(1, std::min(99, (int)(((double)(chainActive.Height() - pindex->nHeight)) / (double)nCheckDepth * (nCheckLevel >= 4 ? 50 : 100)))); if (reportDone < percentageDone/10) { // report every 10% step - LogPrintf("[%d%%]...", percentageDone); + LogPrintf("[%d%%]...", percentageDone); /* Continued */ reportDone = percentageDone/10; } uiInterface.ShowProgress(_("Verifying blocks..."), percentageDone, false); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dbc1760c8..59fdae903 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3076,7 +3076,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve wtxNew.fTimeReceivedIsTxTime = true; wtxNew.fFromMe = true; - LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); + LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); /* Continued */ { // Take key pair from key pool so it won't be used again reservekey.KeepKey(); From d207207fd3ac1a0aaf3c34379f8f02b76dc69c69 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 5 Apr 2018 10:18:26 -0400 Subject: [PATCH 2/2] [logging] add lint-logs.sh to check for newline termination. Check that all calls to LogPrintf() are terminated by a newline, except those that are explicitly marked as 'continued' logs. --- contrib/devtools/lint-logs.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100755 contrib/devtools/lint-logs.sh diff --git a/contrib/devtools/lint-logs.sh b/contrib/devtools/lint-logs.sh new file mode 100755 index 000000000..3bb54359a --- /dev/null +++ b/contrib/devtools/lint-logs.sh @@ -0,0 +1,25 @@ +#!/bin/bash +# +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +# +# Check that all logs are terminated with '\n' +# +# Some logs are continued over multiple lines. They should be explicitly +# commented with \* Continued *\ +# +# There are some instances of LogPrintf() in comments. Those can be +# ignored + + +UNTERMINATED_LOGS=$(git grep "LogPrintf(" -- "*.cpp" | \ + grep -v '\\n"' | \ + grep -v "/\* Continued \*/" | \ + grep -v "LogPrintf()") +if [[ ${UNTERMINATED_LOGS} != "" ]]; then + echo "All calls to LogPrintf() should be terminated with \\n" + echo + echo "${UNTERMINATED_LOGS}" + exit 1 +fi