runtime: Simplify prefs

Signed-off-by: Thomas Habets <thomas@habets.se>
author: Thomas Habets <habets@google.com> 2021-04-06 20:27:56 +0100
committer: mormj <34754695+mormj@users.noreply.github.com> 2021-05-02 19:00:31 -0400
commit: c9482f9a91e2fa58097097be390bcf76ef4c5ba5 (patch)
tree: 30616af4afe24766e20aa35b691aca3474c30a39
parent: 6dbdc285469dd45d3752794c5195bd29ad45a0d5 (diff)
3 files changed, 150 insertions, 173 deletions
diff --git a/gnuradio-runtime/include/gnuradio/prefs.h b/gnuradio-runtime/include/gnuradio/prefs.h
index c2f775a40f..606cf0b1b1 100644
--- a/gnuradio-runtime/include/gnuradio/prefs.h
+++ b/gnuradio-runtime/include/gnuradio/prefs.h
@@ -83,75 +83,78 @@ public:
     /*!
      * \brief Does \p section exist?
      */
-    virtual bool has_section(const std::string& section);
+    bool has_section(const std::string& section);
 
     /*!
      * \brief Does \p option exist?
      */
-    virtual bool has_option(const std::string& section, const std::string& option);
+    bool has_option(const std::string& section, const std::string& option);
 
     /*!
      * \brief If option exists return associated value; else
      * default_val.
      */
-    virtual const std::string get_string(const std::string& section,
-                                         const std::string& option,
-                                         const std::string& default_val);
+    const std::string get_string(const std::string& section,
+                                 const std::string& option,
+                                 const std::string& default_val);
 
     /*!
      * \brief Set or add a string \p option to \p section with value
      * \p val.
      */
-    virtual void set_string(const std::string& section,
-                            const std::string& option,
-                            const std::string& val);
+    void set_string(const std::string& section,
+                    const std::string& option,
+                    const std::string& val);
 
     /*!
      * \brief If option exists and value can be converted to bool,
      * return it; else default_val.
      */
-    virtual bool
+    bool
     get_bool(const std::string& section, const std::string& option, bool default_val);
 
     /*!
      * \brief Set or add a bool \p option to \p section with value \p val.
      */
-    virtual void
-    set_bool(const std::string& section, const std::string& option, bool val);
+    void set_bool(const std::string& section, const std::string& option, bool val);
 
     /*!
      * \brief If option exists and value can be converted to long,
      * return it; else default_val.
      */
-    virtual long
+    long
     get_long(const std::string& section, const std::string& option, long default_val);
 
     /*!
      * \brief Set or add a long \p option to \p section with value \p val.
      */
-    virtual void
-    set_long(const std::string& section, const std::string& option, long val);
+    void set_long(const std::string& section, const std::string& option, long val);
 
     /*!
      * \brief If option exists and value can be converted to double,
      * return it; else default_val.
      */
-    virtual double
+    double
     get_double(const std::string& section, const std::string& option, double default_val);
 
     /*!
      * \brief Set or add a double \p option to \p section with value \p val.
      */
-    virtual void
-    set_double(const std::string& section, const std::string& option, double val);
+    void set_double(const std::string& section, const std::string& option, double val);
 
 protected:
-    virtual std::vector<std::string> _sys_prefs_filenames();
-    virtual void _read_files(const std::vector<std::string>& filenames);
-    virtual char* option_to_env(std::string section, std::string option);
+    std::vector<std::string> _sys_prefs_filenames();
+    void _read_files(const std::vector<std::string>& filenames);
+    char* option_to_env(std::string section, std::string option);
+    template <typename T>
+    T get_general(const std::string& section,
+                  const std::string& option,
+                  const T& default_val);
+    template <typename T>
+    void set_general(const std::string& section, const std::string& option, const T& val);
 
 private:
-    gr::thread::mutex d_mutex;
+    // TODO: add back and actually use: gr::thread::mutex d_mutex;
     config_map_t d_config_map;
 };
 
diff --git a/gnuradio-runtime/lib/prefs.cc b/gnuradio-runtime/lib/prefs.cc
index 5b5557f9cc..14d3fe25fd 100644
--- a/gnuradio-runtime/lib/prefs.cc
+++ b/gnuradio-runtime/lib/prefs.cc
@@ -28,10 +28,22 @@ typedef std::ifstream::char_type char_t;
 
 namespace gr {
 
+namespace {
+
+void stolower(std::string& s)
+{
+    std::transform(s.begin(), s.end(), s.begin(), ::tolower);
+}
+void stoupper(std::string& s)
+{
+    std::transform(s.begin(), s.end(), s.begin(), ::toupper);
+}
+
+} // namespace
+
 prefs* prefs::singleton()
 {
-    static prefs instance; // Guaranteed to be destroyed.
-                           // Instantiated on first use.
+    static prefs instance;
     return &instance;
 }
 
@@ -48,11 +60,9 @@ std::vector<std::string> prefs::_sys_prefs_filenames()
 
     fs::path dir = prefsdir();
     if (fs::is_directory(dir)) {
-        fs::directory_iterator diritr(dir);
-        while (diritr != fs::directory_iterator()) {
-            fs::path p = *diritr++;
-            if (p.extension() == ".conf")
-                fnames.push_back(p.string());
+        for (const auto& p : fs::directory_iterator(dir)) {
+            if (p.path().extension() == ".conf")
+                fnames.push_back(p.path());
         }
         std::sort(fnames.begin(), fnames.end());
     }
@@ -72,48 +82,45 @@ void prefs::_read_files(const std::vector<std::string>& filenames)
 {
     for (const auto& fname : filenames) {
         std::ifstream infile(fname.c_str());
-        if (infile.good()) {
-            try {
-                po::basic_parsed_options<char_t> parsed =
-                    po::parse_config_file(infile, po::options_description(), true);
-                for (const auto& o : parsed.options) {
-                    std::string okey = o.string_key;
-                    size_t pos = okey.find(".");
-                    std::string section, key;
-                    if (pos != std::string::npos) {
-                        section = okey.substr(0, pos);
-                        key = okey.substr(pos + 1);
-                    } else {
-                        section = "default";
-                        key = okey;
-                    }
-                    std::transform(
-                        section.begin(), section.end(), section.begin(), ::tolower);
-                    std::transform(key.begin(), key.end(), key.begin(), ::tolower);
-                    // value of a basic_option is always a std::vector<string>; we only
-                    // allow single values, so:
-                    std::string value = o.value[0];
-                    d_config_map[section][key] = value;
+        if (!infile.good()) {
+            std::cerr << "WARNING: Config file '" << fname
+                      << "' could not be opened for reading." << std::endl;
+            continue;
+        }
+
+        try {
+            po::basic_parsed_options<char_t> parsed =
+                po::parse_config_file(infile, po::options_description(), true);
+            for (const auto& o : parsed.options) {
+                std::string okey = o.string_key;
+                size_t pos = okey.find(".");
+                std::string section, key;
+                if (pos != std::string::npos) {
+                    section = okey.substr(0, pos);
+                    key = okey.substr(pos + 1);
+                } else {
+                    section = "default";
+                    key = okey;
                 }
-            } catch (std::exception& e) {
-                std::cerr << "WARNING: Config file '" << fname
-                          << "' failed to parse:" << std::endl;
-                std::cerr << e.what() << std::endl;
-                std::cerr << "Skipping it" << std::endl;
+                stolower(section);
+                stolower(key);
+                // value of a basic_option is always a std::vector<string>; we only
+                // allow single values, so:
+                std::string value = o.value[0];
+                d_config_map[section][key] = value;
             }
-        } else { // infile.good();
+        } catch (std::exception& e) {
             std::cerr << "WARNING: Config file '" << fname
-                      << "' could not be opened for reading." << std::endl;
+                      << "' failed to parse:" << std::endl;
+            std::cerr << e.what() << std::endl;
+            std::cerr << "Skipping it" << std::endl;
         }
     }
 }
 
 void prefs::add_config_file(const std::string& configfile)
 {
-    std::vector<std::string> filenames;
-    filenames.push_back(configfile);
-
-    _read_files(filenames);
+    _read_files(std::vector<std::string>{ configfile });
 }
 
 
@@ -137,29 +144,28 @@ std::string prefs::to_string()
 
 void prefs::save()
 {
+    // TODO: write the settings in an error-safe way, by writing to a tempfile and
+    // atomically renaming.
     std::string conf = to_string();
     fs::path userconf = fs::path(gr::userconf_path()) / "config.conf";
     std::ofstream fout(userconf);
     fout << conf;
     fout.close();
+    // TODO: throw on error?
 }
 
 char* prefs::option_to_env(std::string section, std::string option)
 {
-    std::stringstream envname;
-    std::string secname = section, optname = option;
-
-    std::transform(section.begin(), section.end(), secname.begin(), ::toupper);
-    std::transform(option.begin(), option.end(), optname.begin(), ::toupper);
-    envname << "GR_CONF_" << secname << "_" << optname;
-
-    return getenv(envname.str().c_str());
+    stoupper(section);
+    stoupper(option);
+    const auto envname = "GR_CONF_" + section + "_" + option;
+    return getenv(envname.c_str());
 }
 
 bool prefs::has_section(const std::string& section)
 {
     std::string s = section;
-    std::transform(section.begin(), section.end(), s.begin(), ::tolower);
+    stolower(s);
     return d_config_map.count(s) > 0;
 }
 
@@ -168,20 +174,20 @@ bool prefs::has_option(const std::string& section, const std::string& option)
     if (option_to_env(section, option))
         return true;
 
-    if (has_section(section)) {
-        std::string s = section;
-        std::transform(section.begin(), section.end(), s.begin(), ::tolower);
-
-        std::string o = option;
-        std::transform(option.begin(), option.end(), o.begin(), ::tolower);
-
-        config_map_itr sec = d_config_map.find(s);
-        return sec->second.count(o) > 0;
-    } else {
+    if (!has_section(section)) {
         return false;
     }
+
+    std::string s = section;
+    stolower(s);
+    std::string o = option;
+    stolower(o);
+
+    config_map_itr sec = d_config_map.find(s);
+    return sec->second.count(o) > 0;
 }
 
+// get_string needs specialization because it can have spaces.
 const std::string prefs::get_string(const std::string& section,
                                     const std::string& option,
                                     const std::string& default_val)
@@ -190,144 +196,112 @@ const std::string prefs::get_string(const std::string& section,
     if (env)
         return std::string(env);
 
-    if (has_option(section, option)) {
-        std::string s = section;
-        std::transform(section.begin(), section.end(), s.begin(), ::tolower);
-
-        std::string o = option;
-        std::transform(option.begin(), option.end(), o.begin(), ::tolower);
-
-        config_map_itr sec = d_config_map.find(s);
-        config_map_elem_itr opt = sec->second.find(o);
-        return opt->second;
-    } else {
+    if (!has_option(section, option)) {
         return default_val;
     }
-}
 
-void prefs::set_string(const std::string& section,
-                       const std::string& option,
-                       const std::string& val)
-{
     std::string s = section;
-    std::transform(section.begin(), section.end(), s.begin(), ::tolower);
+    stolower(s);
 
     std::string o = option;
-    std::transform(option.begin(), option.end(), o.begin(), ::tolower);
+    stolower(o);
 
-    std::map<std::string, std::string> opt_map = d_config_map[s];
-
-    opt_map[o] = val;
-
-    d_config_map[s] = opt_map;
+    config_map_itr sec = d_config_map.find(s);
+    config_map_elem_itr opt = sec->second.find(o);
+    return opt->second;
 }
 
+// get_bool() needs specialization because there are multiple text strings for true/false
 bool prefs::get_bool(const std::string& section,
                      const std::string& option,
                      bool default_val)
 {
-    if (has_option(section, option)) {
-        std::string str = get_string(section, option, "");
-        if (str.empty()) {
-            return default_val;
-        }
-        std::transform(str.begin(), str.end(), str.begin(), ::tolower);
-        if ((str == "true") || (str == "on") || (str == "1"))
-            return true;
-        else if ((str == "false") || (str == "off") || (str == "0"))
-            return false;
-        else
-            return default_val;
-    } else {
+    if (!has_option(section, option)) {
         return default_val;
     }
+    std::string str = get_string(section, option, "");
+    if (str.empty()) {
+        return default_val;
+    }
+    stolower(str);
+    if ((str == "true") || (str == "on") || (str == "1"))
+        return true;
+    if ((str == "false") || (str == "off") || (str == "0"))
+        return false;
+    return default_val;
 }
 
-void prefs::set_bool(const std::string& section, const std::string& option, bool val)
+template <typename T>
+T prefs::get_general(const std::string& section,
+                     const std::string& option,
+                     const T& default_val)
 {
-    std::string s = section;
-    std::transform(section.begin(), section.end(), s.begin(), ::tolower);
+    if (!has_option(section, option)) {
+        return default_val;
+    }
 
-    std::string o = option;
-    std::transform(option.begin(), option.end(), o.begin(), ::tolower);
+    std::string str = get_string(section, option, "");
+    if (str.empty()) {
+        return default_val;
+    }
+    std::stringstream sstr(str);
+    T n;
+    sstr >> n;
+    return n;
+}
 
+template <typename T>
+void prefs::set_general(const std::string& section,
+                        const std::string& option,
+                        const T& val)
+{
+    std::string s = section;
+    stolower(s);
+    std::string o = option;
+    stolower(o);
     std::map<std::string, std::string> opt_map = d_config_map[s];
 
     std::stringstream sstr;
-    sstr << (val == true);
+    sstr << val;
     opt_map[o] = sstr.str();
-
     d_config_map[s] = opt_map;
 }
 
+void prefs::set_bool(const std::string& section, const std::string& option, bool val)
+{
+    return set_general(section, option, val);
+}
+
 long prefs::get_long(const std::string& section,
                      const std::string& option,
                      long default_val)
 {
-    if (has_option(section, option)) {
-        std::string str = get_string(section, option, "");
-        if (str.empty()) {
-            return default_val;
-        }
-        std::stringstream sstr(str);
-        long n;
-        sstr >> n;
-        return n;
-    } else {
-        return default_val;
-    }
+    return get_general(section, option, default_val);
 }
 
 void prefs::set_long(const std::string& section, const std::string& option, long val)
 {
-    std::string s = section;
-    std::transform(section.begin(), section.end(), s.begin(), ::tolower);
-
-    std::string o = option;
-    std::transform(option.begin(), option.end(), o.begin(), ::tolower);
-
-    std::map<std::string, std::string> opt_map = d_config_map[s];
-
-    std::stringstream sstr;
-    sstr << val;
-    opt_map[o] = sstr.str();
-
-    d_config_map[s] = opt_map;
+    return set_general(section, option, val);
 }
 
 double prefs::get_double(const std::string& section,
                          const std::string& option,
                          double default_val)
 {
-    if (has_option(section, option)) {
-        std::string str = get_string(section, option, "");
-        if (str.empty()) {
-            return default_val;
-        }
-        std::stringstream sstr(str);
-        double n;
-        sstr >> n;
-        return n;
-    } else {
-        return default_val;
-    }
+    return get_general(section, option, default_val);
 }
 
 void prefs::set_double(const std::string& section, const std::string& option, double val)
 {
-    std::string s = section;
-    std::transform(section.begin(), section.end(), s.begin(), ::tolower);
-
-    std::string o = option;
-    std::transform(option.begin(), option.end(), o.begin(), ::tolower);
-
-    std::map<std::string, std::string> opt_map = d_config_map[s];
-
-    std::stringstream sstr;
-    sstr << val;
-    opt_map[o] = sstr.str();
+    return set_general(section, option, val);
+}
 
-    d_config_map[s] = opt_map;
+void prefs::set_string(const std::string& section,
+                       const std::string& option,
+                       const std::string& val)
+{
+    return set_general(section, option, val);
 }
 
+
 } /* namespace gr */
diff --git a/gnuradio-runtime/python/gnuradio/gr/bindings/prefs_python.cc b/gnuradio-runtime/python/gnuradio/gr/bindings/prefs_python.cc
index 29a049618d..a9d42dac51 100644
--- a/gnuradio-runtime/python/gnuradio/gr/bindings/prefs_python.cc
+++ b/gnuradio-runtime/python/gnuradio/gr/bindings/prefs_python.cc
@@ -14,7 +14,7 @@
 /* BINDTOOL_GEN_AUTOMATIC(0)                                                       */
 /* BINDTOOL_USE_PYGCCXML(0)                                                        */
 /* BINDTOOL_HEADER_FILE(prefs.h)                                        */
-/* BINDTOOL_HEADER_FILE_HASH(c461d885bc4fd82aa0301248e9f1e958)                     */
+/* BINDTOOL_HEADER_FILE_HASH(59ed43a1f982f3c7da3f0f6b51941294)                     */
 /***********************************************************************************/
 
 #include <pybind11/complex.h>
author	Thomas Habets <habets@google.com>	2021-04-06 20:27:56 +0100
committer	mormj <34754695+mormj@users.noreply.github.com>	2021-05-02 19:00:31 -0400
commit	c9482f9a91e2fa58097097be390bcf76ef4c5ba5 (patch)
tree	30616af4afe24766e20aa35b691aca3474c30a39
parent	6dbdc285469dd45d3752794c5195bd29ad45a0d5 (diff)