Repository: bitcoin/bitcoin
File: src/node/blockstorage.cpp (C++)

Code before:
                LogError("OpenUndoFile failed for %s while writing block undo", pos.ToString());
                return FatalError(m_opts.notifications, state, _("Failed to write undo data."));
            }
            BufferedWriter fileout{file};

            // Write index header
            fileout << GetParams().MessageStart() << blockundo_size;
            pos.nPos += STORAGE_HEADER_BYTES;
            {
                // Calculate checksum
                HashWriter hasher{};
                hasher << block.pprev->GetBlockHash() << blockundo;
                // Write undo data & checksum
                fileout << blockundo << hasher.GetHash();
            }

            fileout.flush(); // Make sure `AutoFile`/`BufferedWriter` go out of scope before we call `FlushUndoFile`
        }

        // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)
        // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height
        // in the block file info as below; note that this does not catch the case where the undo writes are keeping up
        // with the block writes (usually when a synced up node is getting newly mined blocks) -- this case is caught in
        // the FindNextBlockPos function
        if (pos.nFile < cursor.file_num && static_cast<uint32_t>(block.nHeight) == m_blockfile_info[pos.nFile].nHeightLast) {
            // Do not propagate the return code, a failed flush here should not
            // be an indication for a failed write. If it were propagated here,
            // the caller would assume the undo data not to be written, when in
            // fact it is. Note though, that a failed flush might leave the data
            // file untrimmed.
            if (!FlushUndoFile(pos.nFile, true)) {
                LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", pos.nFile);
            }
        } else if (pos.nFile == cursor.file_num && block.nHeight > cursor.undo_height) {
            cursor.undo_height = block.nHeight;
        }
        // update nUndoPos in block index
        block.nUndoPos = pos.nPos;
        block.nStatus |= BLOCK_HAVE_UNDO;
        m_dirty_blockindex.insert(&block);
    }

    return true;
}

bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos, const std::optional<uint256>& expected_hash) const
{
    block.SetNull();

    // Open history file to read
    std::vector<std::byte> block_data;

Review comment (bug):
I'd either delete this now or put it before the actual closing. And do we really need to make this unlikely scenario occupy so much space? And in the other case we simply used `LogError` instead:
```C++
// Make sure that the file is closed before we call `FlushUndoFile`.
if (file.fclose()) {
    LogError("Failed to close block undo file %s: %s", pos.ToString(), SysErrorString(errno));
    return FatalError(m_opts.notifications, state, _("Failed to close block undo file."));
}
```

Code after:
            LogError("OpenUndoFile failed for %s while writing block undo", pos.ToString());
            return FatalError(m_opts.notifications, state, _("Failed to write undo data."));
        }
        {
            BufferedWriter fileout{file};

            // Write index header
            fileout << GetParams().MessageStart() << blockundo_size;
            pos.nPos += STORAGE_HEADER_BYTES;
            {
                // Calculate checksum
                HashWriter hasher{};
                hasher << block.pprev->GetBlockHash() << blockundo;
                // Write undo data & checksum
                fileout << blockundo << hasher.GetHash();
            }
            // BufferedWriter will flush pending data to file when fileout goes out of scope.
        }

        // Make sure that the file is closed before we call `FlushUndoFile`.
        if (file.fclose() != 0) {
            LogError("Failed to close block undo file %s: %s", pos.ToString(), SysErrorString(errno));
            return FatalError(m_opts.notifications, state, _("Failed to close block undo file."));
        }

        // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)
        // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height
        // in the block file info as below; note that this does not catch the case where the undo writes are keeping up
        // with the block writes (usually when a synced up node is getting newly mined blocks) -- this case is caught in
        // the FindNextBlockPos function
        if (pos.nFile < cursor.file_num && static_cast<uint32_t>(block.nHeight) == m_blockfile_info[pos.nFile].nHeightLast) {
            // Do not propagate the return code, a failed flush here should not
            // be an indication for a failed write. If it were propagated here,
            // the caller would assume the undo data not to be written, when in
            // fact it is. Note though, that a failed flush might leave the data
            // file untrimmed.
            if (!FlushUndoFile(pos.nFile, true)) {
                LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", pos.nFile);
            }
        } else if (pos.nFile == cursor.file_num && block.nHeight > cursor.undo_height) {
            cursor.undo_height = block.nHeight;
        }
        // update nUndoPos in block index
        block.nUndoPos = pos.nPos;
        block.nStatus |= BLOCK_HAVE_UNDO;
        m_dirty_blockindex.insert(&block);
    }

    return true;
}