better std::exception logging for CAddrDb #3602

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:addrdb changing 1 files +10 −9

Diapolo commented at 9:53 AM on January 30, 2014: none
- also small logging text changes
Diapolo commented at 8:17 AM on April 4, 2014: none

@sipa I introduced this for other parts of the code already and AFAIK you ACKed it, can you take a short look?
sipa commented at 11:23 PM on April 21, 2014: member

ACK

in src/net.cpp:None in 310b513e21 outdated

1967 |      try {
1968 |          fileout << ssPeers;
1969 |      }
1970 |      catch (std::exception &e) {
1971 | -        return error("CAddrman::Write() : I/O error");
1972 | +        return error("%s : Serialize or I/O error - %s", __PRETTY_FUNCTION__, e.what());

laanwj commented at 10:22 AM on April 22, 2014:

Please use __func__ instead, it is standard (as of C++11) and has been in all c++ compilers for a while. __PRETTY_FUNCTION__ is g++ only.

Diapolo commented at 12:41 PM on April 30, 2014:

Done!

laanwj added this to the milestone 0.9.2 on Apr 23, 2014
laanwj commented at 1:56 PM on April 30, 2014: member

ACK
in src/net.cpp:None in 9889f50bb8 outdated
```
1971 |      fileout.fclose();
1972 |  
1973 |      // replace existing peers.dat, if any, with new peers.dat.XXXX
1974 |      if (!RenameOver(pathTmp, pathAddr))
1975 | -        return error("CAddrman::Write() : Rename-into-place failed");
1976 | +        return error("CAddrDB::Write : Rename-into-place failed");
```
laanwj commented at 1:59 PM on April 30, 2014:

BTW: Any specific reason for using __func__ and %s in some places but keeping the hardcoded function name in others (like here)? It looks a bit inconsistent.

Diapolo commented at 2:03 PM on April 30, 2014:

No, It's just the fact that catch (std::exception &e) mostly uses func etc. and I left it that way,

sipa commented at 2:04 PM on April 30, 2014:

I'd suggest changing it everywhere you can.

laanwj commented at 6:57 AM on May 1, 2014:

Agree with @sipa here. Using __func__ instead of duplicating the function name means that the log messages cannot diverge if the name of the function changes. So it would be good to use it in all cases. But let's at least be consistent within a function
BitcoinPullTester commented at 3:00 PM on April 30, 2014: none
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/9889f50bb885216190ebb3dad9d47ca038a3bc9e for binaries and test log.

This could happen for one of several reasons:
1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
4. The test suite fails on either Linux i386 or Win32
5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
better std::exception logging for CAddrDb
- also small logging text changes
2fdd4c7933
Diapolo commented at 12:50 PM on May 2, 2014: none

@laanwj @sipa Now using func instead of a static function name.
sipa commented at 1:14 PM on May 2, 2014: member

ACK
laanwj merged this on May 2, 2014
laanwj closed this on May 2, 2014
laanwj referenced this in commit 9eb1e1e8ce on May 2, 2014
Diapolo deleted the branch on May 2, 2014
DrahtBot locked this on Sep 8, 2021

Contributors