Make sure LogPrintf strings are line-terminated #6497

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2015_07_terminate_logprintf changing 6 files +10 −9

laanwj commented at 2:44 PM on July 31, 2015: member

Fix the cases where LogPrint[f] was accidentally called without line terminator, which resulted in concatenated log lines.

(see e.g. #6492)
laanwj added the label Docs and Output on Jul 31, 2015
sipa commented at 2:46 PM on July 31, 2015: member

ACK
in src/policy/fees.cpp:None in 5d7fc47cf6 outdated
```
 389 | @@ -390,8 +390,9 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo
 390 |          mapMemPoolTxs[hash].bucketIndex = feeStats.NewTx(txHeight, (double)feeRate.GetFeePerK());
 391 |      }
 392 |      else {
 393 | -        LogPrint("estimatefee", "not adding\n");
 394 | +        LogPrint("estimatefee", "not adding");
 395 |      }
 396 | +    LogPrint("estimatefee", "\n");
```
morcos commented at 3:14 PM on July 31, 2015:

It might be ugly, but it was working correctly before right? This will cause extra new lines I think, because NewTx above prints a new line. I guess we could just remove it from there. This may be a case of too much debugging, I'm not sure if we want all these anyway....
laanwj commented at 1:50 PM on August 3, 2015:

Original code:

LogPrint("estimatefee", "Blockpolicy mempool tx %s ", hash.ToString().substr(0,10)); // Record this as a priority estimate if (entry.GetFee() == 0 || isPriDataPoint(feeRate, curPri)) { ... } // Record this as a fee estimate else if (isFeeDataPoint(feeRate, curPri)) { ... } else { LogPrint("estimatefee", "not adding\n"); }

So the line will not be terminated unless the if() condition falls through.

But if you insist I'll remove this change...
morcos commented at 2:45 PM on August 3, 2015:

I apologize for the ugly code again, but in the if and else if, feeStats and priStats both call NewTx respectively. NewTx adds a new line. I'd suggest leaving your change, but removing the new line in the LogPrint call in NewTx. Or, removing all the LogPrint lines altogether, what are your thoughts about the level of debugging? I found this useful in writing this code and for certain modifications it would be also valuable, but perhaps doesn't have much everyday value. I'm running into the same questions in the mempool limiting code where debugging the various failures is valuable when working on the code but perhaps not when its finished until someone wants to change it.

laanwj commented at 3:26 PM on August 3, 2015:

OK, will do that. This is optional debugging, so I think having it in the first place is fine. It can be useful if someone wants to troubleshoot the fee estimation, which I'm sure has to happen at some point.
jgarzik commented at 4:57 PM on August 1, 2015: contributor

ACK though agree @morcos has a point
Diapolo commented at 2:05 PM on August 3, 2015: none

utACK for trivial-branch

Make sure LogPrintf strings are line-terminated

Fix the cases where LogPrint[f] was accidentally called without line
terminator, which resulted in concatenated log lines.

(see e.g. #6492)

f18b8ec7cf

laanwj force-pushed on Aug 3, 2015
laanwj commented at 3:41 PM on August 3, 2015: member

Ok, updated for @morcos' comment
laanwj merged this on Aug 3, 2015
laanwj closed this on Aug 3, 2015
laanwj referenced this in commit a2bf40dde7 on Aug 3, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#6492 "Read attempted past buffer limit" error while reindexing #6599 Make sure LogPrint strings are line-terminated #7617 [doc/log] Fix markdown syntax and line terminate LogPrint