commit 2b6683fe6b32867cb39a406640d76329d8108f6e
author Willy Tu <wltu@google.com> Sun Feb 20 01:45:42 2022 -0800
committer Willy Tu <wltu@google.com> Tue Feb 22 18:06:07 2022 -0800
tree bdf88c3bef8f87a1e37a2ee0b94328440b221601
parent fdebaa30f75727f0c7c4026024a1a4185f615093

binarystore: Add std::optional for optional config options

The `maxSizeBytes` was using 0 size by default instead of skipping it.
Letting `config.maxSizeBytes` be std::optional allow us to have more
control to see if the 0 value is expected or nullopt.

Change-Id: I128216825609909c489cbd5471b324d88d3aa25b
Signed-off-by: Willy Tu <wltu@google.com>

include/parse_config.hpp

diff --git a/include/parse_config.hpp b/include/parse_config.hpp
index b778923..5bef8ad 100644
--- a/include/parse_config.hpp
+++ b/include/parse_config.hpp
@@ -2,6 +2,7 @@
 
 #include <cstdint>
 #include <nlohmann/json.hpp>
+#include <optional>
 #include <string>
 
 using std::uint32_t;
@@ -12,10 +13,10 @@
 
 struct BinaryBlobConfig
 {
-    std::string blobBaseId;  // Required
-    std::string sysFilePath; // Required
-    uint32_t offsetBytes;    // Optional
-    uint32_t maxSizeBytes;   // Optional
+    std::string blobBaseId;               // Required
+    std::string sysFilePath;              // Required
+    std::optional<uint32_t> offsetBytes;  // Optional
+    std::optional<uint32_t> maxSizeBytes; // Optional
 };
 
 /**
@@ -28,8 +29,19 @@
 {
     j.at("blobBaseId").get_to(config.blobBaseId);
     j.at("sysFilePath").get_to(config.sysFilePath);
-    config.offsetBytes = j.value("offsetBytes", 0);
-    config.maxSizeBytes = j.value("maxSizeBytes", 0);
+    if (j.contains("offsetBytes"))
+    {
+        uint32_t val;
+        j.at("offsetBytes").get_to(val);
+        config.offsetBytes = val;
+    }
+
+    if (j.contains("maxSizeBytes"))
+    {
+        uint32_t val;
+        j.at("maxSizeBytes").get_to(val);
+        config.maxSizeBytes = val;
+    }
 }
 
 } // namespace conf

include/sys_file_impl.hpp

diff --git a/include/sys_file_impl.hpp b/include/sys_file_impl.hpp
index c74c965..fa6f5ec 100644
--- a/include/sys_file_impl.hpp
+++ b/include/sys_file_impl.hpp
@@ -6,6 +6,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 
+#include <optional>
 #include <string>
 
 namespace binstore
@@ -21,7 +22,8 @@
      *     actually reads underlying file at 'offset'
      * @param sys Syscall operation interface
      */
-    explicit SysFileImpl(const std::string& path, size_t offset = 0,
+    explicit SysFileImpl(const std::string& path,
+                         std::optional<size_t> offset = std::nullopt,
                          const internal::Sys* sys = &internal::sys_impl);
     ~SysFileImpl();
     SysFileImpl() = delete;

src/main.cpp

diff --git a/src/main.cpp b/src/main.cpp
index 5cf6c10..bb45f55 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -61,11 +61,13 @@
             return nullptr;
         }
 
-        log<level::INFO>("Loading from config with",
-                         entry("BASE_ID=%s", config.blobBaseId.c_str()),
-                         entry("FILE=%s", config.sysFilePath.c_str()),
-                         entry("MAX_SIZE=%llx", static_cast<unsigned long long>(
-                                                    config.maxSizeBytes)));
+        log<level::INFO>(
+            "Loading from config with",
+            entry("BASE_ID=%s", config.blobBaseId.c_str()),
+            entry("FILE=%s", config.sysFilePath.c_str()),
+            entry("MAX_SIZE=%llx",
+                  static_cast<unsigned long long>(
+                      config.maxSizeBytes ? *config.maxSizeBytes : 0)));
 
         auto file = std::make_unique<binstore::SysFileImpl>(config.sysFilePath,
                                                             config.offsetBytes);

src/sys_file_impl.cpp

diff --git a/src/sys_file_impl.cpp b/src/sys_file_impl.cpp
index 243d82d..9fab76a 100644
--- a/src/sys_file_impl.cpp
+++ b/src/sys_file_impl.cpp
@@ -1,5 +1,6 @@
 #include "sys_file_impl.hpp"
 
+#include <optional>
 #include <system_error>
 
 using namespace std::string_literals;
@@ -19,12 +20,12 @@
 
 } // namespace
 
-SysFileImpl::SysFileImpl(const std::string& path, size_t offset,
+SysFileImpl::SysFileImpl(const std::string& path, std::optional<size_t> offset,
                          const internal::Sys* sys) :
     sys(sys)
 {
     fd_ = sys->open(path.c_str(), O_RDWR);
-    offset_ = offset;
+    offset_ = offset ? *offset : 0;
 
     if (fd_ < 0)
     {

test/parse_config_unittest.cpp

diff --git a/test/parse_config_unittest.cpp b/test/parse_config_unittest.cpp
index b450477..772dcc8 100644
--- a/test/parse_config_unittest.cpp
+++ b/test/parse_config_unittest.cpp
@@ -51,6 +51,16 @@
      {
        "blobBaseId": "/test/",
        "sysFilePath": "/another/path"
+    },
+     {
+       "blobBaseId": "/test/",
+       "sysFilePath": "/another/path",
+       "offsetBytes": 32
+    },
+     {
+       "blobBaseId": "/test/",
+       "sysFilePath": "/another/path",
+       "maxSizeBytes": 32
     }]
   )"_json;
 
@@ -60,7 +70,9 @@
 
         EXPECT_NO_THROW(parseFromConfigFile(element, config));
         EXPECT_EQ(config.blobBaseId, "/test/");
-        EXPECT_TRUE(config.offsetBytes == 32 || config.offsetBytes == 0);
-        EXPECT_TRUE(config.maxSizeBytes == 32 || config.maxSizeBytes == 0);
+        EXPECT_TRUE(config.offsetBytes == std::nullopt ||
+                    *config.offsetBytes == 32);
+        EXPECT_TRUE(config.maxSizeBytes == std::nullopt ||
+                    *config.maxSizeBytes == 32);
     }
 }