Handle unsuccessful fseek(...):s #13149

practicalswift commented at 10:15 AM on May 2, 2018: contributor

Handle unsuccessful fseek(...):s.

Note to reviewers: What is the most appropriate course of actions for each of these unsuccessful fseek(...):s?

Handle unsuccessful fseek(...):s 29c9bdcc14

fanquake added the label Refactoring on May 2, 2018

in src/util.cpp:891 in 29c9bdcc14 outdated

 886 | @@ -887,7 +887,9 @@ void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length) {
 887 |      // Fallback version
 888 |      // TODO: just write one byte per block
 889 |      static const char buf[65536] = {};
 890 | -    fseek(file, offset, SEEK_SET);
 891 | +    if (fseek(file, offset, SEEK_SET)) {
 892 | +        return;

laanwj commented at 1:35 PM on May 2, 2018:

Just returning here is fine - this call is only advisory.

in src/logging.cpp:257 in 29c9bdcc14 outdated

 253 | @@ -254,7 +254,10 @@ void BCLog::Logger::ShrinkDebugFile()
 254 |      {
 255 |          // Restart the file with some of the end
 256 |          std::vector<char> vch(RECENT_DEBUG_HISTORY_SIZE, 0);
 257 | -        fseek(file, -((long)vch.size()), SEEK_END);
 258 | +        if (fseek(file, -((long)vch.size()), SEEK_END)) {

laanwj commented at 1:36 PM on May 2, 2018:

Not sure what should be done here. As-is it will silently fail to shrink the log file but should continue logging otherwise? Maybe log an error message?

jonasschnelli commented at 6:27 AM on May 3, 2018:

Agree with @laanwj: an additional error message would make sense.

in src/index/txindex.cpp:272 in 29c9bdcc14 outdated

 267 | @@ -268,7 +268,9 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe
 268 |      CBlockHeader header;
 269 |      try {
 270 |          file >> header;
 271 | -        fseek(file.Get(), postx.nTxOffset, SEEK_CUR);
 272 | +        if (fseek(file.Get(), postx.nTxOffset, SEEK_CUR)) {
 273 | +            return error("%s: fseek(...) failed", __func__);

laanwj commented at 1:37 PM on May 2, 2018:

LGTM

laanwj approved

jonasschnelli commented at 6:29 AM on May 3, 2018: contributor

utACK 29c9bdcc141afb14fc9e1213f49de4fcded6ce0c (though a LogPrint in ShrinkDebugFile() would be even better).

practicalswift commented at 7:48 AM on May 3, 2018: contributor

@laanwj @jonasschnelli Regarding logging in ShrinkDebugFile(…) – what about the following:

diff --git a/src/logging.cpp b/src/logging.cpp
index 2a55d36..b57b17e 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -255,6 +255,7 @@ void BCLog::Logger::ShrinkDebugFile()
         // Restart the file with some of the end
         std::vector<char> vch(RECENT_DEBUG_HISTORY_SIZE, 0);
         if (fseek(file, -((long)vch.size()), SEEK_END)) {
+            LogPrintf("Failed to shrink debug file: fseek(...) failed\n");
             fclose(file);
             return;
         }

Looks good?

laanwj commented at 12:15 PM on May 3, 2018: member

@practicalswift maybe "debug log file" instead of "debug file"?

Print a log message if we fail to shrink the debug log file 20ce5af4c6

practicalswift commented at 1:44 PM on May 3, 2018: contributor

@laanwj @jonasschnelli Added a commit for the logging. Please re-review :-)

jonasschnelli approved

jonasschnelli commented at 1:46 PM on May 3, 2018: contributor

utACK 20ce5af4c647d9ebd97b40683bd0747383c9dc20

laanwj commented at 2:35 PM on May 3, 2018: member

utACK 20ce5af

laanwj merged this on May 7, 2018

laanwj closed this on May 7, 2018

laanwj referenced this in commit bd83704ec6 on May 7, 2018

jasonbcox referenced this in commit fddf954d94 on Sep 27, 2019

jonspock referenced this in commit 72e7b6df67 on Dec 22, 2019

jonspock referenced this in commit da6fbf6a7c on Dec 22, 2019

jonspock referenced this in commit 508b2aee98 on Dec 22, 2019

jonspock referenced this in commit 44c3e037eb on Dec 23, 2019

proteanx referenced this in commit 6bd1a903c3 on Dec 23, 2019

practicalswift deleted the branch on Apr 10, 2021

UdjinM6 referenced this in commit 39e0a4d46e on May 21, 2021

UdjinM6 referenced this in commit a2d9850e2b on Jun 5, 2021

UdjinM6 referenced this in commit 913079b535 on Jun 5, 2021

TheArbitrator referenced this in commit 38a6e07b66 on Jun 7, 2021

gades referenced this in commit 46c37fb06a on Apr 30, 2022

DrahtBot locked this on Aug 18, 2022