remove printf redefinition from bitcoinrpc.cpp #1977

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:rem_printf_redef_rpc changing 1 files +0 −3
  1. Diapolo commented at 10:26 AM on November 4, 2012: none
    • as the redefiniton of printf happens in util.h, which is included in bitcoinrpc.cpp, we don't need another redefinition
  2. remove printf redefinition from bitcoinrpc.cpp
    - as the redefiniton of printf happens in util.h, which is included in
      bitcoinrpc.cpp, we don't need another redefinition
    ed552cfae0
  3. sipa commented at 10:36 AM on November 4, 2012: member

    Are you sure this is a no-op? Maybe one of the included header files between the undef and the define needs the original, or tries to define printf by itself?

  4. sipa closed this on Nov 4, 2012

  5. sipa reopened this on Nov 4, 2012

  6. Diapolo commented at 10:44 AM on November 4, 2012: none

    It was this pull, who included this change https://github.com/bitcoin/bitcoin/commit/69d605f410c83235aa7b757445e7d0166fcfe2d9, but as you see between the #undef and the #define are only boost or std headers, well I'm not sure it's a no-op, but compilation and running the client is working fine. We include util.h quite often and it's mostly before boost headers... @laanwj It was your pull, perhaps you can enlighten this?

  7. laanwj commented at 11:03 AM on November 4, 2012: member

    That line is actually ancient.

    It's showing up the text as being from me because I renamed src/rpc.cpp to src/bitcoinrpc.cpp. However, the line was in there way before that. Some deeper git forensics shows that the first commit in which the printf redefinition was in src/rpc.cpp was 84c3fb0. This appears to be also a reorganization commit, in which /rpc.cpp was renamed to src/rpc.cpp. Digging even deeper, 22f721d was really the first commit in which it was introduced (by Satoshi himself).

    Looks like it was added for "MSVC 8 compatibility".

    FYI, the git command to grep a while throughout all commits:

     git grep OutputDebugStringF $(git log --pretty=format:%h) -- rpc.cpp
    
  8. Diapolo commented at 11:08 AM on November 4, 2012: none

    Thanks for digging, although I'm still not sure if it's needed there or not. MSVC 8 is ancient, also.

  9. laanwj commented at 11:12 AM on November 4, 2012: member

    No one knows. I'm ok with removing it. After all, as you say, all the rpc*.cpp spinoffs work fine without it.

    Though my real preference would still be to remove the re-definition of printf, rename OutputDebugStringF to some sane name such as debug_log(), and use that throughout the source code. But I don't think everyone agrees.

  10. BitcoinPullTester commented at 4:53 PM on November 4, 2012: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ed552cfae078778ebaebf1786bad0305ee730479 for binaries and test log.

  11. sipa commented at 12:43 AM on November 5, 2012: member

    ACK

  12. laanwj referenced this in commit e88f8887b6 on Nov 10, 2012
  13. laanwj merged this on Nov 10, 2012
  14. laanwj closed this on Nov 10, 2012

  15. laudney referenced this in commit 3cdda4b34c on Mar 19, 2014
  16. KolbyML referenced this in commit bc2ec8590f on Dec 5, 2020
  17. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-21 18:16 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me