replace custom GetFilesize() with boost::filesystem::file_size() #3603

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:file-size changing 3 files +5 −21

Diapolo commented at 9:57 AM on January 30, 2014: none

No description provided.
laanwj commented at 12:53 PM on January 30, 2014: member

ACK
replace custom GetFilesize() with boost::filesystem::file_size() a486abd419
in src/net.cpp:None in 6f44f1cdee outdated
```
1987 | @@ -1986,7 +1988,7 @@ bool CAddrDB::Read(CAddrMan& addr)
1988 |          return error("CAddrman::Read() : open failed");
1989 |  
1990 |      // use file size to size memory buffer
1991 | -    int fileSize = GetFilesize(filein);
1992 | +    int fileSize = (int)boost::filesystem::file_size(pathAddr);
```
laanwj commented at 7:42 AM on January 31, 2014:

This was not introduced in this pull but we really shouldn't be using int for file sizes but off_t for compatibility with 64 bit files (sure, if the address database is more than 2GB I'd be afraid as well... but having negative values here due to wraparound can cause bad stuff).

Diapolo commented at 8:12 AM on January 31, 2014:

The Boost function returns a boost::uintmax_t, why not use uint64_t seems more common in our code already.

laanwj commented at 8:23 AM on January 31, 2014:

boost::uintmax_t sounds like overkill. This is the largest integer type supported on the architecture. This may be 128 bit on some systems. You shouldn't blindly switch to an unsigned type here. This will break things, see if ( dataSize < 0 ) dataSize = 0; below.

int64_t would be possible, but off_t is the official type for this in C, and sizes according to the size of file offsets in the API (see https://stackoverflow.com/questions/1035657/seeking-and-reading-large-files-in-a-linux-c-application).

in src/util.cpp:None in 6f44f1cdee outdated

1193 | @@ -1204,7 +1194,7 @@ void ShrinkDebugFile()
1194 |      // Scroll debug.log if it's getting too big
1195 |      boost::filesystem::path pathLog = GetDataDir() / "debug.log";
1196 |      FILE* file = fopen(pathLog.string().c_str(), "r");
1197 | -    if (file && GetFilesize(file) > 10 * 1000000)
1198 | +    if (file && (int)boost::filesystem::file_size(pathLog) > 10 * 1000000)

laanwj commented at 8:28 AM on February 10, 2014:

No need to cast to int here. The comparison is valid either way.

Diapolo commented at 3:34 PM on February 10, 2014:

Removed the cast in both cases...

BitcoinPullTester commented at 11:01 PM on March 3, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/a486abd419d76ec3aec4adb90216eccb7b4be0c4 for binaries and test log. 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.
laanwj referenced this in commit ebb783a9f2 on Mar 27, 2014
laanwj merged this on Mar 27, 2014
laanwj closed this on Mar 27, 2014
Diapolo deleted the branch on Mar 27, 2014
DrahtBot locked this on Sep 8, 2021

Contributors

Diapolo

laanwj

BitcoinPullTester