Fix LogPrint to LogPrintf #8230

1. 
   thelazier commented at 5:47 pm on June 20, 2016: contributor

   Printing Log without category defined should use LogPrintf
2. Fix LogPrint to LogPrintf

   Printing Log without category defined should use LogPrintf

   ba6194928a
3. 
   eduffield222 commented at 9:08 pm on June 20, 2016: none

   utACK
4. 
   pstratem commented at 9:14 pm on June 20, 2016: contributor

   utACK ba6194928a03a474b442744511da4c753441469d
5. 
   laanwj commented at 8:08 am on June 21, 2016: member

   utACK ba61949 Good catch! This mistake is all too easy to make, unfortunately.

   At some point we either need to rename one of those functions, or create an automatic checker for Printf/LogPrintf arguments, or both.

6. laanwj added the label Docs and Output on Jun 21, 2016
7. laanwj added the label Bug on Jun 21, 2016
8. laanwj merged this on Jun 21, 2016
9. laanwj closed this on Jun 21, 2016

   ---

10. laanwj referenced this in commit f3eebcf515 on Jun 21, 2016
11. 
    gmaxwell commented at 8:10 am on June 21, 2016: contributor

    I hate that this isn’t typesafe, what is this, PHP? :) … when I saw the ticket open I felt immediately stupid, sure than it must have been me that did this again. (posthumous utACK).
12. 
    laanwj commented at 8:18 am on June 21, 2016: member

    Eh, why is this merging into 0.12 though? Isn’t this a problem on master?

    I hate that this isn’t typesafe, what is this, PHP? :) .

    Well it was a step up from C’s printf - none of this will allow mangling values or corrupting memory. It is typesafe, e.g you don’t need size specifiers, it doesn’t matter if you use %i or %lli etc. Worst-case, you get an exception if the number of arguments doesn’t match the number of %. Or nothing is logged and you rub your head why. There would be logging methods that are more robust, e.g. C++/Qt style:

    0LogInfo() << "";
    1LogDebug("rpc") << "Invalid RPC call " << strMethod;
    

    (tinyformat even uses this internally, but emulates C printf-isms because that was what was already in the code and this had the least impact)

    Then again, there’s so much to do already, it’s never been urgent.

13. laanwj referenced this in commit bf9c70b100 on Jun 21, 2016
14. 
    laanwj commented at 8:22 am on June 21, 2016: member

    Forward-ported to master as bf9c70b. (this is not how we usually work, please base your PR on master next time unless it’s a backport)
15. 
    sipa commented at 8:24 am on June 21, 2016: member

    Changing the debug category into an enum would also avoid type ambiguity (which arises from the fact that the format string and the debug category are both strings).
16. 
    laanwj commented at 8:29 am on June 21, 2016: member

    Changing the debug category into an enum would also avoid type ambiguity

    That’s a neat idea, a problem with that is that it requires centralizing all the debug categories of the program in one place. That would be inimical to modularization.

    Another option would be to create a DebugCategory type that is based on string, and only allow passing that as category argument.

    0DebugCategory LOG_RPC("rpc");
    1...
    2LogPrintf(LOG_RPC, "Invalid RPC call %s\n", strMethod);
    

17. thelazier deleted the branch on Jun 21, 2016
18. DrahtBot locked this on Sep 8, 2021

Contributors
thelazier eduffield222 pstratem laanwj gmaxwell sipa

Labels
Bug Docs