Managing codes handle and file with unique ptrs

Author: cducher

src/nwp_emulator/grib_file_reader.cc

@@ -13,7 +13,6 @@
 #include <sstream>
 #include <iostream>
 #include <typeinfo>
-#include <variant>
 
 #include "eckit/exception/Exceptions.h"
 #include "eckit/log/Log.h"
@@ -125,7 +124,7 @@ void GRIBFileReader::setupReader(const eckit::PathName& inputPath, bool isRoot)
     char buffer[64];
     size_t size = sizeof(buffer);
     // 1. Lock the grid name from the passed GRIB file
-    if (codes_get_string(grib_, "gridName", buffer, &size) == GRIB_NOT_FOUND) {
+    if (codes_get_string(grib(), "gridName", buffer, &size) == GRIB_NOT_FOUND) {
         eckit::Log::error() << "Grid type unsupported at the moment, exit" << std::endl;
         eckit::mpi::comm().abort(1);
     }
@@ -135,15 +134,11 @@ void GRIBFileReader::setupReader(const eckit::PathName& inputPath, bool isRoot)
     std::string paramMd, _;
     eckit::Log::info() << "Params (name,levtype,level) : ";
     for (int i = 0; i < count_ - 1; ++i) {
-        int err;
         readMsgMetadata(_, paramMd);
         params_.push_back(paramMd);
         eckit::Log::info() << params_[i] << "; ";
-        codes_handle_delete(grib_);
-        grib_ = codes_handle_new_from_file(nullptr, currentFile_, PRODUCT_GRIB, &err);
-        if (!grib_) {
-            eckit::Log::error() << "Could not create grib handle. Error: " << err << " Check source file " << srcPath
-                                << std::endl;
+        if (!resetGribHandle()) {
+            eckit::Log::error() << " Check source file " << srcPath << std::endl;
             eckit::mpi::comm().abort(1);
         }
     }
@@ -172,19 +167,15 @@ void GRIBFileReader::validateSrcFiles(bool isRoot) {
         fileParams.clear();
         openGribFile(filename, true);
         for (int i = 0; i < count_ - 1; ++i) {
-            int err;
             readMsgMetadata(gridName, paramMd);
             if (gridName != gridName_) {
                 eckit::Log::error() << "Grid in " << filename << " is different from setup ("
                                     << gridName_ << "), exit..." << std::endl;
                 eckit::mpi::comm().abort(1);
             }
             fileParams.push_back(paramMd);
-            codes_handle_delete(grib_);
-            grib_ = codes_handle_new_from_file(nullptr, currentFile_, PRODUCT_GRIB, &err);
-            if (!grib_) {
-                eckit::Log::error() << "Could not create grib handle. Error: " << err << " Check source file "
-                                    << filename << std::endl;
+            if (!resetGribHandle()) {
+                eckit::Log::error() << " Check source file " << filename << std::endl;
                 eckit::mpi::comm().abort(1);
             }
         }
@@ -210,7 +201,7 @@ void GRIBFileReader::readMsgMetadata(std::string& gridName, std::string& paramMd
     char buffer[64];
     size_t size    = sizeof(buffer);
     gridName       = "";
-    int gridStatus = codes_get_string(grib_, "gridName", buffer, &size);
+    int gridStatus = codes_get_string(grib(), "gridName", buffer, &size);
     if (gridStatus != GRIB_NOT_FOUND) {
         gridName = std::string(buffer);
     }
@@ -219,7 +210,7 @@ void GRIBFileReader::readMsgMetadata(std::string& gridName, std::string& paramMd
     for (const auto& keyword : keywords) {
         std::memset(buffer, 0, 64);
         size = sizeof(buffer);
-        codes_get_string(grib_, keyword, buffer, &size);
+        codes_get_string(grib(), keyword, buffer, &size);
         paramMd.append(buffer);
         paramMd.append(",");  // separator
     }
@@ -234,7 +225,7 @@ bool GRIBFileReader::openGribFile(const eckit::PathName& path, bool exit) {
     // No check for existence here because the paths passed belong to the src dir
     // It is assumed they will not be deleted during runtime
     // No check for file format as checked during construction
-    currentFile_ = fopen(path.asString().c_str(), "rb");
+    currentFile_.reset(fopen(path.asString().c_str(), "rb"));
     if (!currentFile_) {
         eckit::Log::error() << "Could not open file " << path << std::endl;
         if (exit) {
@@ -243,31 +234,33 @@ bool GRIBFileReader::openGribFile(const eckit::PathName& path, bool exit) {
         return false;  // defer error handling
     }
     index_ = 0;
-    codes_count_in_file(nullptr, currentFile_, &count_);
-    int err;
-    grib_ = codes_handle_new_from_file(nullptr, currentFile_, PRODUCT_GRIB, &err);
-    if (!grib_) {
-        eckit::Log::error() << "Could not create grib handle. Error: " << err << std::endl;
+    codes_count_in_file(nullptr, currentFile(), &count_);
+    if (!resetGribHandle()) {
         if (exit) {
             eckit::mpi::comm().abort(1);
         }
-        return false;  // defer error handling
+        return false;
     }
     return true;
 }
 
-void GRIBFileReader::closeGribFile() {
-    if (grib_) {
-        codes_handle_delete(grib_);
-    }
-    if (currentFile_) {
-        fclose(currentFile_);
+bool GRIBFileReader::resetGribHandle() {
+    int err;
+    // This calls the deleter of the previous handle first
+    grib_.reset(codes_handle_new_from_file(nullptr, currentFile(), PRODUCT_GRIB, &err));
+    if (!grib_) {
+        eckit::Log::error() << "Could not create grib handle. Error: " << err << std::endl;
+        return false;
     }
+    return true;
+}
+
+void GRIBFileReader::closeGribFile() {
     count_ = 0;
     index_ = 0;
-    // To avoid segfault at destruction if they have already been dereferenced
-    currentFile_ = nullptr;
-    grib_        = nullptr;
+    // Calls the custom destructors
+    currentFile_.reset(nullptr);
+    grib_.reset(nullptr);
 }
 
 bool GRIBFileReader::done() {
@@ -307,28 +300,21 @@ bool GRIBFileReader::nextMessage(std::string& shortName, std::string& levtype, s
         std::getline(ss, level, ',');
 
         size_t nbOfValues;
-        codes_get_size(grib_, "values", &nbOfValues);
+        codes_get_size(grib(), "values", &nbOfValues);
         data.resize(nbOfValues);
         if constexpr (std::is_same_v<FIELD_TYPE_REAL, double>) {
             ASSERT((std::is_same_v<FIELD_TYPE_REAL, double>));
-            codes_get_double_array(grib_, "values", reinterpret_cast<double*>(data.data()), &nbOfValues);
+            codes_get_double_array(grib(), "values", reinterpret_cast<double*>(data.data()), &nbOfValues);
         }
         else {
             ASSERT((std::is_same_v<FIELD_TYPE_REAL, float>));
-            codes_get_float_array(grib_, "values", reinterpret_cast<float*>(data.data()), &nbOfValues);
+            codes_get_float_array(grib(), "values", reinterpret_cast<float*>(data.data()), &nbOfValues);
         }
     }
     ++index_;
     eckit::mpi::comm().broadcast(count_, root_);
     if (rank_ == root_ && index_ < count_) {
-        // Load the next message
-        int err;
-        codes_handle_delete(grib_);
-        grib_ = codes_handle_new_from_file(nullptr, currentFile_, PRODUCT_GRIB, &err);
-        if (!grib_) {
-            eckit::Log::error() << "Could not create grib handle. Error: " << err << " Skipping..." << std::endl;
-            return false;
-        }
+        return resetGribHandle(); // Load the next message
     }
     return true;
 }

src/nwp_emulator/grib_file_reader.h

@@ -12,12 +12,33 @@
 
 #include <string>
 #include <vector>
+#include <cstdio>
+#include <memory>
 
 #include "eccodes.h"
 #include "eckit/filesystem/PathName.h"
 
 #include "data_reader.h"
 
+/**
+ * @brief A structure for a custom deleter to manage codes handles.
+ * 
+ * This deleter allows to use codes handles pointers as unique pointers for safer memory usage.
+ */
+struct codesHandleDeleter {
+    void operator()(codes_handle* h) { if (h) {codes_handle_delete(h);} }
+};
+
+/**
+ * @brief A structure for a custom deleter to manage files.
+ * 
+ * This deleter allows to use C-style file pointers as unique pointers for safer memory usage.
+ * `std::fstream` is not compatible with eccodes, hence the need to stick to `FILE`.
+ */
+struct fileDeleter {
+    void operator()(FILE* f) { if (f) {fclose(f);} }
+};
+
 namespace nwp_emulator {
 
 /**
@@ -34,8 +55,8 @@ namespace nwp_emulator {
 class GRIBFileReader final : public DataReader {
 private:
     std::vector<eckit::PathName> srcFilenames_;
-    FILE* currentFile_  = nullptr;
-    codes_handle* grib_ = nullptr;
+    std::unique_ptr<FILE, fileDeleter> currentFile_{nullptr, fileDeleter()};
+    std::unique_ptr<codes_handle, codesHandleDeleter> grib_{nullptr, codesHandleDeleter()};
 
     size_t rank_;
     size_t root_;
@@ -55,6 +76,22 @@ class GRIBFileReader final : public DataReader {
     /// Closes current file, and resets pointers.
     void closeGribFile();
 
+    /// Gets the grib handle pointer.
+    codes_handle* grib() { return grib_.get(); }
+
+    /// Gets the file pointer.
+    FILE* currentFile() { return currentFile_.get(); }
+
+    /**
+     * @brief Resets the GRIB handle. Decodes the next message in the current file.
+     * 
+     * This method first resets the handle pointer using the deleter, making sure memory is properly deallocated.
+     * 
+     * @return True if successful, false if there was an error loading the next message or if the current file
+     * has had all its messages decoded already.
+     */
+    bool resetGribHandle();
+
     /// Called from the constructor by the main reader
     void setupReader(const eckit::PathName& path, bool isRoot);