From b5e6398c960d1f4f7281fb6fc02b22b0c70ca4aa Mon Sep 17 00:00:00 2001
From: Marcus Holland-Moritz <github@mhxnet.de>
Date: Fri, 5 Jan 2024 18:50:16 +0100
Subject: [PATCH] refactor: add parse_image_offset + test

---
 include/dwarfs/util.h      |  2 ++
 src/dwarfs/util.cpp        | 23 +++++++++++++++++++++++
 src/dwarfs_main.cpp        | 10 +---------
 src/dwarfsck_main.cpp      |  9 +--------
 src/dwarfsextract_main.cpp |  8 +-------
 test/utils_test.cpp        | 14 ++++++++++++++
 6 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/include/dwarfs/util.h b/include/dwarfs/util.h
index c190578c..d36649d9 100644
--- a/include/dwarfs/util.h
+++ b/include/dwarfs/util.h
@@ -40,6 +40,8 @@ size_t parse_size_with_unit(std::string const& str);
 std::chrono::milliseconds parse_time_with_unit(std::string const& str);
 std::chrono::system_clock::time_point parse_time_point(std::string const& str);
 
+file_off_t parse_image_offset(std::string const& str);
+
 inline std::u8string string_to_u8string(std::string const& in) {
   return std::u8string(reinterpret_cast<char8_t const*>(in.data()), in.size());
 }
diff --git a/src/dwarfs/util.cpp b/src/dwarfs/util.cpp
index c37913c7..65b298f4 100644
--- a/src/dwarfs/util.cpp
+++ b/src/dwarfs/util.cpp
@@ -32,9 +32,11 @@
 
 #include <date/date.h>
 
+#include <folly/Conv.h>
 #include <folly/String.h>
 
 #include "dwarfs/error.h"
+#include "dwarfs/options.h"
 #include "dwarfs/util.h"
 
 extern "C" int dwarfs_wcwidth(int ucs);
@@ -170,6 +172,27 @@ std::chrono::system_clock::time_point parse_time_point(std::string const& str) {
   DWARFS_THROW(runtime_error, "cannot parse time point");
 }
 
+file_off_t parse_image_offset(std::string const& str) {
+  if (str == "auto") {
+    return filesystem_options::IMAGE_OFFSET_AUTO;
+  }
+
+  auto off = folly::tryTo<file_off_t>(str);
+
+  if (!off) {
+    auto ce = folly::makeConversionError(off.error(), str);
+    DWARFS_THROW(runtime_error,
+                 fmt::format("failed to parse image offset: {} ({})", str,
+                             folly::exceptionStr(ce)));
+  }
+
+  if (off.value() < 0) {
+    DWARFS_THROW(runtime_error, "image offset must be positive");
+  }
+
+  return off.value();
+}
+
 std::string sys_string_to_string(sys_string const& in) {
 #ifdef _WIN32
   std::u16string tmp(in.size(), 0);
diff --git a/src/dwarfs_main.cpp b/src/dwarfs_main.cpp
index 891ea1b5..b59e2b76 100644
--- a/src/dwarfs_main.cpp
+++ b/src/dwarfs_main.cpp
@@ -1192,15 +1192,7 @@ void load_filesystem(dwarfs_userdata& userdata) {
   fsopts.inode_offset = inode_offset;
 
   if (opts.image_offset_str) {
-    std::string image_offset{opts.image_offset_str};
-
-    try {
-      fsopts.image_offset = image_offset == "auto"
-                                ? filesystem_options::IMAGE_OFFSET_AUTO
-                                : folly::to<file_off_t>(image_offset);
-    } catch (...) {
-      DWARFS_THROW(runtime_error, "failed to parse offset: " + image_offset);
-    }
+    fsopts.image_offset = parse_image_offset(opts.image_offset_str);
   }
 
   std::unordered_set<std::string> perfmon_enabled;
diff --git a/src/dwarfsck_main.cpp b/src/dwarfsck_main.cpp
index d3422b85..24bd4d1b 100644
--- a/src/dwarfsck_main.cpp
+++ b/src/dwarfsck_main.cpp
@@ -141,14 +141,7 @@ int dwarfsck_main(int argc, sys_char** argv, iolayer const& iol) {
 
     fsopts.metadata.enable_nlink = true;
     fsopts.metadata.check_consistency = check_integrity;
-
-    try {
-      fsopts.image_offset = image_offset == "auto"
-                                ? filesystem_options::IMAGE_OFFSET_AUTO
-                                : folly::to<file_off_t>(image_offset);
-    } catch (...) {
-      DWARFS_THROW(runtime_error, "failed to parse offset: " + image_offset);
-    }
+    fsopts.image_offset = parse_image_offset(image_offset);
 
     std::shared_ptr<mmif> mm = iol.os->map_file(input);
 
diff --git a/src/dwarfsextract_main.cpp b/src/dwarfsextract_main.cpp
index 4b91ce22..969bfabe 100644
--- a/src/dwarfsextract_main.cpp
+++ b/src/dwarfsextract_main.cpp
@@ -119,14 +119,8 @@ int dwarfsextract_main(int argc, sys_char** argv, iolayer const& iol) {
     auto level = logger::parse_level(log_level);
     stream_logger lgr(iol.term, iol.err, level, level >= logger::DEBUG);
     filesystem_options fsopts;
-    try {
-      fsopts.image_offset = image_offset == "auto"
-                                ? filesystem_options::IMAGE_OFFSET_AUTO
-                                : folly::to<file_off_t>(image_offset);
-    } catch (...) {
-      DWARFS_THROW(runtime_error, "failed to parse offset: " + image_offset);
-    }
 
+    fsopts.image_offset = parse_image_offset(image_offset);
     fsopts.block_cache.max_bytes = parse_size_with_unit(cache_size_str);
     fsopts.block_cache.num_workers = num_workers;
     fsopts.block_cache.disable_block_integrity_check = disable_integrity_check;
diff --git a/test/utils_test.cpp b/test/utils_test.cpp
index 22138ba3..3537f0ac 100644
--- a/test/utils_test.cpp
+++ b/test/utils_test.cpp
@@ -29,6 +29,7 @@
 
 #include "dwarfs/error.h"
 #include "dwarfs/offset_cache.h"
+#include "dwarfs/options.h"
 #include "dwarfs/util.h"
 
 using namespace dwarfs;
@@ -379,3 +380,16 @@ TEST(utils, parse_time_point) {
               ::testing::ThrowsMessage<dwarfs::runtime_error>(
                   ::testing::HasSubstr("cannot parse time point")));
 }
+
+TEST(utils, parse_image_offset) {
+  EXPECT_EQ(0, parse_image_offset("0"));
+  EXPECT_EQ(1, parse_image_offset("1"));
+  EXPECT_EQ(1024, parse_image_offset("1024"));
+  EXPECT_EQ(filesystem_options::IMAGE_OFFSET_AUTO, parse_image_offset("auto"));
+  EXPECT_THAT([] { parse_image_offset("-1"); },
+              ::testing::ThrowsMessage<dwarfs::runtime_error>(
+                  ::testing::HasSubstr("image offset must be positive")));
+  EXPECT_THAT([] { parse_image_offset("asd"); },
+              ::testing::ThrowsMessage<dwarfs::runtime_error>(
+                  ::testing::HasSubstr("failed to parse image offset")));
+}