HDF5: File constructor: Allow read-only, drop SWMR

Read-only: It's far too easy to corrupt an HDF5 file. Making it
possible to open a file in read-only mode is an important step
towards making that not quite as easy.

SWMR: As explained in a comment, we can enable it in read-only mode by
default; in read-and-write mode it can be enabled one the constructor
returns, through the function swmrEnable().

As a result of this change, the line

  HDF5::File file(filename, true);

which was legal in the past but not used by anyone to the best of my
knowledge, will no longer compile.

dune/fufem/hdf5/file.hh

@@ -27,11 +27,15 @@ public:
   }
 
   virtual hid_t c_obj() = 0;
+  virtual bool isReadOnly() const = 0;
 };
 
+enum class Access { READONLY, READWRITE };
+
 class File : public Grouplike {
 public:
-  File(std::string filename, bool swmr = false) {
+  File(std::string filename, Access access = Access::READWRITE)
+      : readOnly_(access == Access::READONLY) {
     struct stat buffer;
 
     /* Use the latest HDF5 format.
@@ -44,29 +48,27 @@ public:
     H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST);
 
     bool const fileExists = stat(filename.c_str(), &buffer) == 0;
-
-    /*
-      Files are currently opened for reading and writing. This seems
-      reasonable for now because C++ is far more likely to be involved
-      in the generation of HDF5 data than in their processing.
-
-      Given how easy it is to corrupt HDF5 data through unexpected
-      process termination, however, it might make sense to allow
-      read-only access in the future
-    */
-    unsigned int flags = fileExists ? H5F_ACC_RDWR : H5F_ACC_EXCL;
-    if (swmr) {
+    bool const createFile = !fileExists;
+    if (createFile && readOnly_)
+      DUNE_THROW(
+          Dune::Exception,
+          "Tried to open non-existing file in read-only mode: " << filename);
+
+    // If createFile is true: We already know the file does not exist,
+    // so H5F_ACC_EXCL and H5F_ACC_TRUNC are essentially equivalent
+    unsigned int flags =
+        createFile ? H5F_ACC_EXCL : (readOnly_ ? H5F_ACC_RDONLY : H5F_ACC_RDWR);
 #if H5_VERSION_GE(1, 10, 0)
-      flags |= H5F_ACC_SWMR_WRITE;
-#else
-      std::cerr
-          << "WARNING: SWMR requested but not supported by this version of HDF5"
-          << std::endl;
+    // Enabling SWMR reading by default on HDF-1.10 does not hurt
+    // (except maybe performance). In contrast, enabling SWMR writing
+    // produces a file that is not readable by HDF5-1.8. The user can
+    // still enable SWMR writing later through swmrEnable().
+    if (readOnly_)
+      flags |= H5F_ACC_SWMR_READ;
 #endif
-    }
 
-    file_ = fileExists ? H5Fopen(filename.c_str(), flags, fapl)
-                       : H5Fcreate(filename.c_str(), flags, H5P_DEFAULT, fapl);
+    file_ = createFile ? H5Fcreate(filename.c_str(), flags, H5P_DEFAULT, fapl)
+                       : H5Fopen(filename.c_str(), flags, fapl);
     if (file_ < 0)
       DUNE_THROW(Dune::Exception, "Unable to open file: " << filename);
   }
@@ -76,15 +78,20 @@ public:
   ~File() { H5Fclose(file_); }
 
   hid_t c_obj() override { return file_; }
-
+  bool isReadOnly() const override { return readOnly_; }
   bool swmrEnabled() { return swmrEnabled_; }
   void swmrEnable() {
     if (swmrEnabled_)
       return;
 
 #if H5_VERSION_GE(1, 10, 0)
-    swmrEnabled_ = true;
-    H5Fstart_swmr_write(file_);
+    if (!readOnly_) {
+      swmrEnabled_ = true;
+      H5Fstart_swmr_write(file_);
+    } else {
+      std::cerr << "WARNING: SWMR writing requested for a read-only file"
+                << std::endl;
+    }
 #else
     std::cerr
         << "WARNING: SWMR requested but not supported by this version of HDF5"
@@ -94,6 +101,7 @@ public:
 
 private:
   hid_t file_;
+  bool const readOnly_;
   bool swmrEnabled_ = false;
 };
 
@@ -109,6 +117,7 @@ public:
   ~Group() { H5Gclose(group_); }
 
   hid_t c_obj() override { return group_; }
+  bool isReadOnly() const override { return parent_.isReadOnly(); }
 
 private:
   Grouplike &parent_;

dune/fufem/hdf5/sequenceio.hh

@@ -36,6 +36,9 @@ public:
       checkDimensions();
       checkChunkSize();
     } else {
+      if (file_.isReadOnly())
+        DUNE_THROW(Dune::Exception, "Dataset not found: " << datasetname);
+
       auto const initial_dims = maxExtent(0);
       auto const max_dims = maxExtent(H5S_UNLIMITED);
       hid_t file_space =

dune/fufem/hdf5/singletonwriter.hh

@@ -25,6 +25,8 @@ public:
       : file_(file), capacity_{{T(args)...}} {
     static_assert(sizeof...(args) == spatialDimensions,
                   "wrong number of arguments");
+    if (file_.isReadOnly())
+        DUNE_THROW(Dune::Exception, "Cannot handle read-only file");
 
     if (file_.hasDataset(datasetname)) {
       dset_ = file_.openDataset(datasetname);

dune/fufem/test/test-hdf5.cc

@@ -23,6 +23,44 @@ void deleteIfNecessary(std::string filename) {
   }
 }
 
+template <class Vector>
+bool checkMissingFile(Vector const &written0) {
+  bool passed = true;
+  using ft = typename Vector::block_type::field_type;
+  std::string const filename("no-such-file.h5");
+  try {
+    deleteIfNecessary(filename);
+    HDF5::File file(filename.c_str(), HDF5::Access::READONLY);
+    HDF5::SequenceIO<2, ft> h5writer(file, "data", 2, 3);
+    addEntry(h5writer, 0, written0);
+  } catch (Dune::Exception e) {
+  } // Expecting an exception here!
+  return passed;
+}
+
+template <class Vector>
+bool checkMissingDataset(Vector const &written0) {
+  bool passed = true;
+  using ft = typename Vector::block_type::field_type;
+  std::string const filename("out-missing.h5");
+  {
+    deleteIfNecessary(filename);
+    HDF5::File file(filename.c_str());
+    HDF5::SequenceIO<2, ft> h5writer(file, "data", 2, 3);
+    addEntry(h5writer, 0, written0);
+  }
+
+  Vector read0;
+  try {
+    HDF5::File file(filename.c_str(), HDF5::Access::READONLY);
+    HDF5::SequenceIO<2, ft> h5reader(file, "dataX", 2, 3);
+    readEntry(h5reader, 0, read0);
+    passed = false;
+  } catch (Dune::Exception e) {
+  } // Expecting an exception here!
+  return passed;
+}
+
 template <class Vector>
 bool checkAddSequence(Vector const &written0, Vector const &written1) {
   bool passed = true;
@@ -42,7 +80,7 @@ bool checkAddSequence(Vector const &written0, Vector const &written1) {
   Vector read0;
   Vector read1;
   {
-    HDF5::File file(filename.c_str());
+    HDF5::File file(filename.c_str(), HDF5::Access::READONLY);
     HDF5::SequenceIO<2, ft> h5reader(file, "data", 2, 3);
     readEntry(h5reader, 0, read0);
     readEntry(h5reader, 1, read1);
@@ -122,10 +160,13 @@ bool testParameters() {
   deleteIfNecessary(filename);
   HDF5::File file(filename);
 
+  std::cout << "Content of dataset:" << std::endl
+            << "===============" << std::endl;
   Dune::ParameterTree parset;
   parset["a"] = "3";
   parset["c.d"] = "something";
   parset.report();
+  std::cout << "===============" << std::endl;
 
   traverseParset(parset, "",
                  [&](std::string key, std::string value, std::string prefix) {
@@ -153,9 +194,10 @@ int main(int argc, char *argv[]) {
   };
   Vector b1 = {{0, 10, 20}, {30, 40, 50}};
 
-
   bool passed = true;
   passed = passed and testParameters();
+  passed = passed and checkMissingFile(b0);
+  passed = passed and checkMissingDataset(b0);
   passed = passed and checkAddSequence(b0, b1);
 #if H5_VERSION_GE(1, 10, 0)
   passed = passed and checkAppendSequence(b0, b1);