Cleanup config and add tests

src/config.cpp

@@ -17,27 +17,17 @@
 */
 
 #include "config.hpp"
-#include "ef.hpp"
 #include "errno.hpp"
-#include "from_string.hpp"
+#include "fmt/core.h"
 #include "fs_path.hpp"
 #include "nonstd/string.hpp"
-#include "num.hpp"
-#include "rwlock.hpp"
 #include "str.hpp"
-#include "to_string.hpp"
 #include "version.hpp"
 
-#include <algorithm>
-#include <cstdint>
 #include <fstream>
-#include <iostream>
 #include <string>
 
-#include <pthread.h>
 #include <string.h>
-#include <sys/stat.h>
-#include <unistd.h>
 
 
 constexpr static const char CACHE_FILES_PROCESS_NAMES_DEFAULT[] =
@@ -61,6 +51,7 @@ Config::CfgConfigFile::from_string(const std::string_view s_)
   if(_depth > 5)
     return -ELOOP;
   _depth++;
+  struct DepthGuard { int &d; ~DepthGuard() { --d; } } guard{_depth};
 
   cfg_file = (s_.empty() ? _cfg_file : s_);
 
@@ -68,8 +59,6 @@ Config::CfgConfigFile::from_string(const std::string_view s_)
   if(rv == 0)
     _cfg_file = cfg_file;
 
-  _depth--;
-
   return rv;
 }
 
@@ -316,37 +305,6 @@ Config::has_key(const std::string &key_) const
   return _map.count(key_);
 }
 
-void
-Config::keys(std::string &s_) const
-{
-  s_.reserve(512);
-
-  for(const auto &[key,val] : _map)
-    {
-      s_ += key;
-      s_ += '\0';
-    }
-
-  if(!s_.empty())
-    s_.resize(s_.size() - 1);
-}
-
-
-void
-Config::keys_xattr(std::string &s_) const
-{
-  s_.reserve(1024);
-  for(const auto &[key,val] : _map)
-    {
-      if(val->display == false)
-        continue;
-
-      s_ += "user.mergerfs.";
-      s_ += key;
-      s_ += '\0';
-    }
-}
-
 ssize_t
 Config::keys_listxattr_size() const
 {
@@ -380,13 +338,16 @@ Config::keys_listxattr(char   *list_,
       if(val->display == false)
         continue;
 
-      auto rv = fmt::format_to_n(list,size,
-                                 "user.mergerfs.{}\0",
-                                 key);
-      if(rv.out > (list + size))
+      static constexpr std::string_view prefix = "user.mergerfs.";
+      ssize_t entry_size = (ssize_t)(prefix.size() + key.size() + 1);
+      if(entry_size > size)
         return -ERANGE;
-      list += rv.size;
-      size -= rv.size;
+      memcpy(list, prefix.data(), prefix.size());
+      list += prefix.size();
+      memcpy(list, key.data(), key.size());
+      list += key.size();
+      *list++ = '\0';
+      size -= entry_size;
     }
 
   return (list - list_);
@@ -442,7 +403,6 @@ Config::set(const std::string &kv_)
 int
 Config::from_stream(std::istream &istrm_)
 {
-  int rv;
   std::string line;
   Config::ErrVec new_errs;
 
@@ -452,23 +412,21 @@ Config::from_stream(std::istream &istrm_)
       if(line.empty() || (line[0] == '#'))
         continue;
 
-      rv = set(line);
+      int rv = set(line);
       if(rv < 0)
         new_errs.push_back({-rv,line});
     }
 
-  rv = (new_errs.empty() ? 0 : -EINVAL);
   errs.insert(errs.end(),
               new_errs.begin(),
               new_errs.end());
 
-  return rv;
+  return (new_errs.empty() ? 0 : -EINVAL);
 }
 
 int
 Config::from_file(const std::string &filepath_)
 {
-  int rv;
   std::ifstream ifstrm;
 
   errno = 0;
@@ -480,11 +438,7 @@ Config::from_file(const std::string &filepath_)
       return -errno;
     }
 
-  rv = from_stream(ifstrm);
-
-  ifstrm.close();
-
-  return rv;
+  return from_stream(ifstrm);
 }
 
 void
@@ -517,10 +471,10 @@ Config::is_cmd_xattr(const std::string_view &attrname_)
   return nonstd::starts_with(attrname_,"user.mergerfs.cmd.");
 }
 
-std::string
-Config::prune_ctrl_xattr(const std::string &s_)
+std::string_view
+Config::prune_ctrl_xattr(const std::string_view s_)
 {
-  const size_t offset = (sizeof("user.mergerfs.") - 1);
+  constexpr size_t offset = (sizeof("user.mergerfs.") - 1);
 
   if(offset < s_.size())
     return s_.substr(offset);

src/config.hpp

@@ -72,7 +72,6 @@ typedef ToFromWrapper<fs::path>             ConfigPath;
 typedef std::map<std::string,ToFromString*> Str2TFStrMap;
 typedef ROToFromWrapper<std::string>        ConfigROSTR;
 
-extern const std::string CONTROLFILE;
 
 class Config
 {
@@ -108,9 +107,6 @@ public:
 public:
   Config();
 
-public:
-  Config& operator=(const Config&);
-
 public:
   ConfigBOOL     allow_idmap;
   ConfigBOOL     async_read;
@@ -201,8 +197,6 @@ public:
 
 public:
   bool has_key(const std::string &key) const;
-  void keys(std::string &s) const;
-  void keys_xattr(std::string &s) const;
   ssize_t keys_listxattr(char *list, size_t size) const;
   ssize_t keys_listxattr_size() const;
   const Str2TFStrMap& get_map() const { return _map; }
@@ -221,13 +215,11 @@ public:
   static bool is_ctrl_file(const fs::path &fusepath);
   static bool is_mergerfs_xattr(const char *attrname);
   static bool is_cmd_xattr(const std::string_view &attrname);
-  static std::string prune_ctrl_xattr(const std::string &s);
+  static std::string_view prune_ctrl_xattr(const std::string_view s);
   static std::string_view prune_cmd_xattr(const std::string_view &s);
 
 private:
   Str2TFStrMap _map;
 };
 
-std::ostream& operator<<(std::ostream &s,const Config::ErrVec &ev);
-
 extern Config cfg;

tests/tests.cpp

@@ -6,9 +6,11 @@
 
 #include <atomic>
 #include <chrono>
+#include <cstring>
 #include <future>
 #include <mutex>
 #include <numeric>
+#include <sstream>
 #include <thread>
 
 template<typename Predicate>
@@ -1521,6 +1523,418 @@ test_tp_heavy_add_remove_churn_under_enqueue()
   TEST_CHECK(executed.load() == TOTAL);
 }
 
+void
+test_config_has_key()
+{
+  Config cfg;
+
+  TEST_CHECK(cfg.has_key("async-read") == true);
+  TEST_CHECK(cfg.has_key("branches") == true);
+  TEST_CHECK(cfg.has_key("version") == true);
+  TEST_CHECK(cfg.has_key("nonexistent-key-xyz") == false);
+  TEST_CHECK(cfg.has_key("") == false);
+
+  // Underscore form should NOT match (has_key does no normalization)
+  TEST_CHECK(cfg.has_key("async_read") == false);
+}
+
+void
+test_config_keys()
+{
+  Config cfg;
+
+  const auto &map = cfg.get_map();
+
+  // Result must be non-empty
+  TEST_CHECK(!map.empty());
+
+  // Verify known keys are present
+  TEST_CHECK(map.count("async-read") > 0);
+  TEST_CHECK(map.count("branches")   > 0);
+  TEST_CHECK(map.count("version")    > 0);
+
+  // No key should be empty
+  for(const auto &[k, v] : map)
+    TEST_CHECK(!k.empty());
+}
+
+void
+test_config_keys_xattr()
+{
+  Config cfg;
+
+  ssize_t needed = cfg.keys_listxattr(nullptr, 0);
+  TEST_CHECK(needed > 0);
+
+  std::string s(needed, '\0');
+  ssize_t written = cfg.keys_listxattr(s.data(), needed);
+  TEST_CHECK(written > 0);
+  s.resize(written);
+
+  // Every token must start with "user.mergerfs." and end with NUL
+  const char *p   = s.data();
+  const char *end = p + s.size();
+  int count = 0;
+
+  while(p < end)
+    {
+      const char *nul = static_cast<const char*>(memchr(p, '\0', end - p));
+      TEST_CHECK(nul != nullptr);
+      if(!nul) break;
+
+      std::string entry(p, nul);
+      TEST_CHECK(str::startswith(entry, "user.mergerfs."));
+      ++count;
+      p = nul + 1;
+    }
+
+  TEST_CHECK(count > 0);
+}
+
+void
+test_config_keys_listxattr_size()
+{
+  Config cfg;
+
+  ssize_t sz = cfg.keys_listxattr_size();
+  TEST_CHECK(sz > 0);
+
+  // Size must accommodate a buffer filled by keys_listxattr
+  std::string buf(sz, '\0');
+  ssize_t written = cfg.keys_listxattr(buf.data(), sz);
+  TEST_CHECK(written > 0);
+  TEST_CHECK(written <= sz);
+}
+
+void
+test_config_keys_listxattr_fill()
+{
+  Config cfg;
+
+  // size=0 must return required size (not write anything)
+  ssize_t needed = cfg.keys_listxattr(nullptr, 0);
+  TEST_CHECK(needed > 0);
+
+  // Allocate exact size and fill
+  std::vector<char> buf(needed);
+  ssize_t written = cfg.keys_listxattr(buf.data(), needed);
+  TEST_CHECK(written > 0);
+  TEST_CHECK(written <= needed);
+
+  // Every entry must start with "user.mergerfs." and end with NUL
+  const char *p   = buf.data();
+  const char *end = p + written;
+  int count = 0;
+  while(p < end)
+    {
+      const char *nul = static_cast<const char*>(memchr(p, '\0', end - p));
+      TEST_CHECK(nul != nullptr);
+      if(!nul) break;
+
+      std::string entry(p, nul);
+      TEST_CHECK(str::startswith(entry, "user.mergerfs."));
+      TEST_CHECK(entry.size() > strlen("user.mergerfs."));
+      ++count;
+      p = nul + 1;
+    }
+
+  TEST_CHECK(count > 0);
+
+  // Keys reported by keys_listxattr must be consistent across two calls
+  std::vector<char> xattr_buf(needed);
+  ssize_t xattr_written = cfg.keys_listxattr(xattr_buf.data(), needed);
+  TEST_CHECK(xattr_written == written);
+  TEST_CHECK(memcmp(buf.data(), xattr_buf.data(), written) == 0);
+}
+
+void
+test_config_keys_listxattr_erange()
+{
+  Config cfg;
+
+  // Buffer too small (1 byte) must return -ERANGE
+  char tiny[1] = {'\xff'};
+  ssize_t rv = cfg.keys_listxattr(tiny, 1);
+  TEST_CHECK(rv == -ERANGE);
+}
+
+void
+test_config_get_set_roundtrip()
+{
+  Config cfg;
+  std::string val;
+
+  // bool: async-read
+  TEST_CHECK(cfg.set("async-read", "false") == 0);
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+
+  TEST_CHECK(cfg.set("async-read", "true") == 0);
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "true");
+
+  // u64: cache-attr
+  TEST_CHECK(cfg.set("cache.attr", "42") == 0);
+  TEST_CHECK(cfg.get("cache.attr", &val) == 0);
+  TEST_CHECK(val == "42");
+
+  // string: fsname
+  TEST_CHECK(cfg.set("fsname", "myfs") == 0);
+  TEST_CHECK(cfg.get("fsname", &val) == 0);
+  TEST_CHECK(val == "myfs");
+}
+
+void
+test_config_get_unknown_key()
+{
+  Config cfg;
+  std::string val;
+
+  int rv = cfg.get("no-such-key", &val);
+  TEST_CHECK(rv == -ENOATTR);
+}
+
+void
+test_config_set_unknown_key()
+{
+  Config cfg;
+
+  int rv = cfg.set("no-such-key", "value");
+  TEST_CHECK(rv == -ENOATTR);
+}
+
+void
+test_config_set_invalid_value()
+{
+  Config cfg;
+  std::string val;
+
+  // "async-read" is a bool — "notabool" must be rejected
+  int rv = cfg.set("async-read", "notabool");
+  TEST_CHECK(rv == -EINVAL);
+
+  // value must be unchanged
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "true");
+}
+
+void
+test_config_set_underscore_normalization()
+{
+  Config cfg;
+  std::string val;
+
+  // Underscores must be converted to hyphens
+  TEST_CHECK(cfg.set("async_read", "false") == 0);
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+}
+
+void
+test_config_get_underscore_normalization()
+{
+  Config cfg;
+  std::string val;
+
+  TEST_CHECK(cfg.set("async-read", "false") == 0);
+  TEST_CHECK(cfg.get("async_read", &val) == 0);
+  TEST_CHECK(val == "false");
+}
+
+void
+test_config_set_kv_form()
+{
+  Config cfg;
+  std::string val;
+
+  TEST_CHECK(cfg.set("async-read=false") == 0);
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+
+  // With spaces around delimiter
+  TEST_CHECK(cfg.set("async-read = true") == 0);
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "true");
+}
+
+void
+test_config_readonly_before_init()
+{
+  Config cfg;
+  std::string val;
+
+  // version is RO even before finish_initializing()
+  int rv = cfg.set("version", "9.9.9");
+  TEST_CHECK(rv == -EROFS);
+}
+
+void
+test_config_readonly_after_init()
+{
+  Config cfg;
+  std::string val;
+
+  // Before init: mutable fields can be set
+  TEST_CHECK(cfg.set("async-read", "false") == 0);
+
+  cfg.finish_initializing();
+
+  // async-read is marked RO after init
+  int rv = cfg.set("async-read", "true");
+  TEST_CHECK(rv == -EROFS);
+
+  // Value must be unchanged
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+}
+
+void
+test_config_mutable_after_init()
+{
+  Config cfg;
+  std::string val;
+
+  cfg.finish_initializing();
+
+  // dropcacheonclose is NOT RO after init — should still be settable
+  TEST_CHECK(cfg.set("dropcacheonclose", "true") == 0);
+  TEST_CHECK(cfg.get("dropcacheonclose", &val) == 0);
+  TEST_CHECK(val == "true");
+}
+
+void
+test_config_from_stream_basic()
+{
+  Config cfg;
+  std::string val;
+
+  std::istringstream ss("async-read=false\ncache.attr=99\n");
+  int rv = cfg.from_stream(ss);
+  TEST_CHECK(rv == 0);
+
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+
+  TEST_CHECK(cfg.get("cache.attr", &val) == 0);
+  TEST_CHECK(val == "99");
+}
+
+void
+test_config_from_stream_comments_and_blanks()
+{
+  Config cfg;
+  std::string val;
+
+  std::istringstream ss(
+    "# this is a comment\n"
+    "\n"
+    "   \n"
+    "async-read=false\n"
+    "# another comment\n"
+    "cache.attr=7\n"
+  );
+
+  int rv = cfg.from_stream(ss);
+  TEST_CHECK(rv == 0);
+
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+
+  TEST_CHECK(cfg.get("cache.attr", &val) == 0);
+  TEST_CHECK(val == "7");
+}
+
+void
+test_config_from_stream_bad_lines()
+{
+  Config cfg;
+
+  std::istringstream ss("async-read=false\nbad-key-xyz=oops\ncache.attr=5\n");
+  int rv = cfg.from_stream(ss);
+
+  // Must report error but still apply valid lines
+  TEST_CHECK(rv == -EINVAL);
+  TEST_CHECK(!cfg.errs.empty());
+
+  std::string val;
+  TEST_CHECK(cfg.get("async-read", &val) == 0);
+  TEST_CHECK(val == "false");
+
+  TEST_CHECK(cfg.get("cache.attr", &val) == 0);
+  TEST_CHECK(val == "5");
+}
+
+void
+test_config_from_file_nonexistent()
+{
+  Config cfg;
+
+  int rv = cfg.from_file("/this/path/does/not/exist/config.txt");
+  TEST_CHECK(rv < 0);
+  TEST_CHECK(!cfg.errs.empty());
+}
+
+void
+test_config_is_rootdir()
+{
+  TEST_CHECK(Config::is_rootdir("") == true);
+  TEST_CHECK(Config::is_rootdir("/") == false);
+  TEST_CHECK(Config::is_rootdir("/foo") == false);
+}
+
+void
+test_config_is_ctrl_file()
+{
+  TEST_CHECK(Config::is_ctrl_file(".mergerfs") == true);
+  TEST_CHECK(Config::is_ctrl_file("/.mergerfs") == false);
+  TEST_CHECK(Config::is_ctrl_file("mergerfs") == false);
+  TEST_CHECK(Config::is_ctrl_file("") == false);
+}
+
+void
+test_config_is_mergerfs_xattr()
+{
+  TEST_CHECK(Config::is_mergerfs_xattr("user.mergerfs.async-read") == true);
+  TEST_CHECK(Config::is_mergerfs_xattr("user.mergerfs.") == true);
+  TEST_CHECK(Config::is_mergerfs_xattr("user.mergerfs") == false);
+  TEST_CHECK(Config::is_mergerfs_xattr("user.other.async-read") == false);
+  TEST_CHECK(Config::is_mergerfs_xattr("") == false);
+}
+
+void
+test_config_is_cmd_xattr()
+{
+  TEST_CHECK(Config::is_cmd_xattr("user.mergerfs.cmd.foo") == true);
+  TEST_CHECK(Config::is_cmd_xattr("user.mergerfs.cmd.") == true);
+  TEST_CHECK(Config::is_cmd_xattr("user.mergerfs.async-read") == false);
+  TEST_CHECK(Config::is_cmd_xattr("user.mergerfs.cmd") == false);
+  TEST_CHECK(Config::is_cmd_xattr("") == false);
+}
+
+void
+test_config_prune_ctrl_xattr()
+{
+  TEST_CHECK(Config::prune_ctrl_xattr("user.mergerfs.async-read") == "async-read");
+  TEST_CHECK(Config::prune_ctrl_xattr("user.mergerfs.cache.attr") == "cache.attr");
+  // Exactly the prefix — nothing after it: returns empty
+  TEST_CHECK(Config::prune_ctrl_xattr("user.mergerfs.") == "");
+  // Shorter than prefix — returns empty
+  TEST_CHECK(Config::prune_ctrl_xattr("user.mergerfs") == "");
+  TEST_CHECK(Config::prune_ctrl_xattr("") == "");
+}
+
+void
+test_config_prune_cmd_xattr()
+{
+  using sv = std::string_view;
+
+  TEST_CHECK(Config::prune_cmd_xattr("user.mergerfs.cmd.foo") == sv("foo"));
+  TEST_CHECK(Config::prune_cmd_xattr("user.mergerfs.cmd.bar.baz") == sv("bar.baz"));
+  TEST_CHECK(Config::prune_cmd_xattr("user.mergerfs.cmd.") == sv(""));
+  TEST_CHECK(Config::prune_cmd_xattr("user.mergerfs.cmd") == sv(""));
+  TEST_CHECK(Config::prune_cmd_xattr("") == sv(""));
+}
+
 TEST_LIST =
   {
    {"nop",test_nop},
@@ -1538,6 +1952,32 @@ TEST_LIST =
    {"config_statfsignore",test_config_statfs_ignore},
    {"config_xattr",test_config_xattr},
    {"config",test_config},
+   {"config_has_key",test_config_has_key},
+   {"config_keys",test_config_keys},
+   {"config_keys_xattr",test_config_keys_xattr},
+   {"config_keys_listxattr_size",test_config_keys_listxattr_size},
+   {"config_keys_listxattr_fill",test_config_keys_listxattr_fill},
+   {"config_keys_listxattr_erange",test_config_keys_listxattr_erange},
+   {"config_get_set_roundtrip",test_config_get_set_roundtrip},
+   {"config_get_unknown_key",test_config_get_unknown_key},
+   {"config_set_unknown_key",test_config_set_unknown_key},
+   {"config_set_invalid_value",test_config_set_invalid_value},
+   {"config_set_underscore_normalization",test_config_set_underscore_normalization},
+   {"config_get_underscore_normalization",test_config_get_underscore_normalization},
+   {"config_set_kv_form",test_config_set_kv_form},
+   {"config_readonly_before_init",test_config_readonly_before_init},
+   {"config_readonly_after_init",test_config_readonly_after_init},
+   {"config_mutable_after_init",test_config_mutable_after_init},
+   {"config_from_stream_basic",test_config_from_stream_basic},
+   {"config_from_stream_comments_and_blanks",test_config_from_stream_comments_and_blanks},
+   {"config_from_stream_bad_lines",test_config_from_stream_bad_lines},
+   {"config_from_file_nonexistent",test_config_from_file_nonexistent},
+   {"config_is_rootdir",test_config_is_rootdir},
+   {"config_is_ctrl_file",test_config_is_ctrl_file},
+   {"config_is_mergerfs_xattr",test_config_is_mergerfs_xattr},
+   {"config_is_cmd_xattr",test_config_is_cmd_xattr},
+   {"config_prune_ctrl_xattr",test_config_prune_ctrl_xattr},
+   {"config_prune_cmd_xattr",test_config_prune_cmd_xattr},
    {"str",test_str_stuff},
    {"tp_construct_default",test_tp_construct_default},
    {"tp_construct_named",test_tp_construct_named},