Avoid file overwriting in fallback AllocateFileRange implementation #33164

1. 
   hebasto commented at 8:38 pm on August 9, 2025: member

   On the master branch, the fallback variant of AllocateFileRange, introduced in #1677, overwrites the file’s content. This causes issues on some systems during some “reindex” scenarios. Additionally, the recently introduced feature_reindex_init.py test is also broken on these systems.

   The affected systems include: OpenBSD, NetBSD, OmniOS, OpenIndiana.

   This PR avoids such overwriting.

   Fixes #33128 and feature_reindex_init.py test on affected systems.

2. Avoid file overwriting in fallback `AllocateFileRange` implementation 6528709c87
3. hebasto added the label Block storage on Aug 9, 2025
4. 
   DrahtBot commented at 8:38 pm on August 9, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33164.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. in src/util/fs_helpers.cpp:209 in 6528709c87

   205@@ -206,17 +206,28 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
   206     if (0 == posix_fallocate(fileno(file), 0, nEndPos)) return;
   207 #endif
   208     // Fallback version
   209-    // TODO: just write one byte per block
   

   ---

   ---

   

   luke-jr commented at 12:43 pm on August 11, 2025:

   Why lose this?
6. in src/util/fs_helpers.cpp:212 in 6528709c87

   210-    static const char buf[65536] = {};
   211-    if (fseek(file, offset, SEEK_SET)) {
   212+    if (fseek(file, 0, SEEK_END)) {
   213+        return;
   214+    }
   215+    const int64_t filesize = std::ftell(file);
   

   ---

   ---

   

   luke-jr commented at 12:45 pm on August 11, 2025:

   It looks safe to mix std::ftell with C file i/o functions, but is there any reason to do so?
7. in src/util/fs_helpers.cpp:219 in 6528709c87

   218+    }
   219+    if (offset + length <= static_cast<unsigned int>(filesize)) {
   220         return;
   221     }
   222-    while (length > 0) {
   223+    unsigned int inc_size = offset + length - static_cast<unsigned int>(filesize);
   

   ---

   ---

   

   luke-jr commented at 12:46 pm on August 11, 2025:

   Probably better to use size_t or at least long

   ---

   

   luke-jr commented at 2:29 pm on August 11, 2025:

   Change of behaviour: This can allocate before offset if the file is smaller. If that’s desired behaviour, there’s no point to the offset parameter at all…?
8. in src/util/fs_helpers.cpp:222 in 6528709c87

   221     }
   222-    while (length > 0) {
   223+    unsigned int inc_size = offset + length - static_cast<unsigned int>(filesize);
   224+    if (fseek(file, filesize, SEEK_SET)) {
   225+        return;
   226+    }
   

   ---

   ---

   

   luke-jr commented at 2:30 pm on August 11, 2025:

   Redundant?
9. luke-jr changes_requested
10. 
    luke-jr commented at 2:32 pm on August 11, 2025: member

    The description for AllocateFileRange says “the range specified in the arguments will never contain live data”, so I’m not sure this is the right approach.

Contributors
hebasto DrahtBot luke-jr

Labels
Block storage