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

practicalswift · 2018-05-02T10:15:34Z

Handle unsuccessful fseek(...):s.

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

laanwj · 2018-05-02T13:35:04Z

src/util.cpp

    static const char buf[65536] = {};
-    fseek(file, offset, SEEK_SET);
+    if (fseek(file, offset, SEEK_SET)) {
+        return;


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

laanwj · 2018-05-02T13:36:20Z

src/logging.cpp

        // Restart the file with some of the end
        std::vector<char> vch(RECENT_DEBUG_HISTORY_SIZE, 0);
-        fseek(file, -((long)vch.size()), SEEK_END);
+        if (fseek(file, -((long)vch.size()), SEEK_END)) {


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?

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

laanwj · 2018-05-02T13:37:19Z

src/index/txindex.cpp

        file >> header;
-        fseek(file.Get(), postx.nTxOffset, SEEK_CUR);
+        if (fseek(file.Get(), postx.nTxOffset, SEEK_CUR)) {
+            return error("%s: fseek(...) failed", __func__);


jonasschnelli · 2018-05-03T06:29:10Z

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

practicalswift · 2018-05-03T07:48:43Z

@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 · 2018-05-03T12:15:23Z

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

practicalswift · 2018-05-03T13:44:42Z

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

jonasschnelli

utACK 20ce5af

laanwj · 2018-05-03T14:35:36Z

utACK 20ce5af

20ce5af Print a log message if we fail to shrink the debug log file (practicalswift) 29c9bdc Handle unsuccessful fseek(...):s (practicalswift) Pull request description: Handle unsuccessful `fseek(...)`:s. **Note to reviewers:** What is the most appropriate course of actions for each of these unsuccessful `fseek(...)`:s? Tree-SHA512: 5b3d82dbdd15d434d3f08dcb4df62888da4df8541d2586f56a4e529083005f6782c39e10645acd1ec403da83061bbfd8dbf2dddc66e09268d410ad0918c61876

Summary: 20ce5af Print a log message if we fail to shrink the debug log file (practicalswift) 29c9bdc Handle unsuccessful fseek(...):s (practicalswift) Pull request description: Handle unsuccessful `fseek(...)`:s. **Note to reviewers:** What is the most appropriate course of actions for each of these unsuccessful `fseek(...)`:s? Tree-SHA512: 5b3d82dbdd15d434d3f08dcb4df62888da4df8541d2586f56a4e529083005f6782c39e10645acd1ec403da83061bbfd8dbf2dddc66e09268d410ad0918c61876 Backport of Core PR13149 bitcoin/bitcoin#13149 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4021

Summary: 20ce5af Print a log message if we fail to shrink the debug log file (practicalswift) 29c9bdc Handle unsuccessful fseek(...):s (practicalswift) Pull request description: Handle unsuccessful `fseek(...)`:s. **Note to reviewers:** What is the most appropriate course of actions for each of these unsuccessful `fseek(...)`:s? Tree-SHA512: 5b3d82dbdd15d434d3f08dcb4df62888da4df8541d2586f56a4e529083005f6782c39e10645acd1ec403da83061bbfd8dbf2dddc66e09268d410ad0918c61876 Backport of Core PR13149 bitcoin/bitcoin#13149 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4021

20ce5af Print a log message if we fail to shrink the debug log file (practicalswift) 29c9bdc Handle unsuccessful fseek(...):s (practicalswift) Pull request description: Handle unsuccessful `fseek(...)`:s. **Note to reviewers:** What is the most appropriate course of actions for each of these unsuccessful `fseek(...)`:s? Tree-SHA512: 5b3d82dbdd15d434d3f08dcb4df62888da4df8541d2586f56a4e529083005f6782c39e10645acd1ec403da83061bbfd8dbf2dddc66e09268d410ad0918c61876

Handle unsuccessful fseek(...):s

29c9bdc

fanquake added the Refactoring label May 2, 2018

laanwj approved these changes May 2, 2018

View reviewed changes

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

20ce5af

jonasschnelli approved these changes May 3, 2018

View reviewed changes

laanwj merged commit 20ce5af into bitcoin:master May 7, 2018

Bushstar mentioned this pull request May 8, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#327

Merged

luke-jr mentioned this pull request Jul 13, 2018

0.16: Remaining backports for 0.16.2 #13644

Merged

practicalswift deleted the fseek branch April 10, 2021 19:34

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

practicalswift commented May 2, 2018 •

edited

Loading

Uh oh!

laanwj May 2, 2018

Uh oh!

laanwj May 2, 2018

Uh oh!

jonasschnelli May 3, 2018

Uh oh!

laanwj May 2, 2018

Uh oh!

jonasschnelli commented May 3, 2018

Uh oh!

practicalswift commented May 3, 2018 •

edited

Loading

Uh oh!

laanwj commented May 3, 2018

Uh oh!

practicalswift commented May 3, 2018

Uh oh!

jonasschnelli left a comment

Uh oh!

laanwj commented May 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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

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

Uh oh!

Conversation

practicalswift commented May 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj May 2, 2018

Choose a reason for hiding this comment

Uh oh!

laanwj May 2, 2018

Choose a reason for hiding this comment

Uh oh!

jonasschnelli May 3, 2018

Choose a reason for hiding this comment

Uh oh!

laanwj May 2, 2018

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented May 3, 2018

Uh oh!

practicalswift commented May 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented May 3, 2018

Uh oh!

practicalswift commented May 3, 2018

Uh oh!

jonasschnelli left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj commented May 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

practicalswift commented May 2, 2018 •

edited

Loading

practicalswift commented May 3, 2018 •

edited

Loading