Cleaning and refactoring block fs for readability

[dafeda]

Read about block_fs and did some clean-up.

Pre review checklist

• Added appropriate release note label
• PR title captures the intent of the changes, and is fitting for release notes.
• Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

[eivindjahren]

I had a reread parts of "clean code" last night so take this as a highly influenced opinion. Perhaps the block should be extracted as a function?

[dafeda]

Can the function be re-used you think?

[eivindjahren]

No, the style used by Robert C. Martin is shorter functions (that do one thing at one abstraction level) where the name serves as documentation, ie.:

static std::tuple<int, int> read_mount_file_header(const fs::path &mount_file){
    FILE *stream = util_fopen(mount_file.c_str(), "r");
    int id = util_fread_int(stream);
    int version = util_fread_int(stream);
    fclose(stream);
    return {id, version};
}

static void validate_mount_file(const fs::path &mount_file){
    auto &[id, version] = read_mount_file_header(mount_file);
    if (version != 0)
        throw std::runtime_error(fmt::format(
            "block_fs data version unexpected. Expected 0, got {}", version));
    if (id != MOUNT_MAP_MAGIC_INT)
        util_abort("%s: The file:%s does not seem to be a valid block_fs "
                   "mount map \n",
                   __func__, mount_file.c_str());
}

static block_fs_type *block_fs_alloc_empty(const fs::path &mount_file,
                                           int block_size, int fsync_interval,
                                           bool read_only) {
    block_fs_type *block_fs = new block_fs_type;
    UTIL_TYPE_ID_INIT(block_fs, BLOCK_FS_TYPE_ID);

    block_fs->fsync_interval = fsync_interval;
    block_fs->block_size = block_size;

    validate_mount_file(mount_file);

    block_fs_reinit(block_fs);

    block_fs->data_owner = !read_only;
    return block_fs;
}

[eivindjahren]

Also, should that second util_abort also be a throw std::runtime_error?

[eivindjahren]

The changes are good. There is more we could do, but I leave it up to you whether you want that it a separate PR.

[codecov-commenter]

Codecov Report

Merging #3552 (6c1ca98) into main (e8a7eeb) will increase coverage by 0.01%.
The diff coverage is 48.21%.

@@            Coverage Diff             @@
##             main    #3552      +/-   ##
==========================================
+ Coverage   64.99%   65.01%   +0.01%     
==========================================
  Files         599      599              
  Lines       46993    46989       -4     
  Branches     4216     4210       -6     
==========================================
+ Hits        30544    30549       +5     
+ Misses      15071    15065       -6     
+ Partials     1378     1375       -3     

Impacted Files	Coverage Δ
libres/lib/res_util/block_fs.cpp	68.11% <48.21%> (+2.05%)	⬆️
libres/lib/enkf/block_fs_driver.cpp	79.73% <0.00%> (+0.65%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more