From a475dd15e65759f2d20822cd3b19b874575c7f5e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 25 Apr 2014 16:09:26 -0700 Subject: [PATCH] Migrate universal variables to env_var_t structure. Encapsulate universal variable storage into a class for better testability. --- env.cpp | 29 ++------ env.h | 13 ++++ env_universal.cpp | 10 +-- env_universal.h | 3 +- env_universal_common.cpp | 151 +++++++++++++++++++++++---------------- env_universal_common.h | 31 +++++++- fishd.cpp | 3 +- 7 files changed, 147 insertions(+), 93 deletions(-) diff --git a/env.cpp b/env.cpp index f349f5b1a..7a0fc85b4 100644 --- a/env.cpp +++ b/env.cpp @@ -80,20 +80,6 @@ extern char **environ; */ extern char **__environ; -/** - A variable entry. Stores the value of a variable and whether it - should be exported. Obviously, it needs to be allocated large - enough to fit the value string. -*/ -struct var_entry_t -{ - wcstring val; /**< The value of the variable */ - bool exportv; /**< Whether the variable should be exported */ - - var_entry_t() : exportv(false) { } -}; - -typedef std::map var_table_t; bool g_log_forks = false; bool g_use_posix_spawn = false; //will usually be set to true @@ -824,7 +810,7 @@ int env_set(const wcstring &key, const wchar_t *val, int var_mode) env_universal_barrier(); } - if (env_universal_get(key)) + if (! env_universal_get(key).missing()) { bool exportv; if (var_mode & ENV_EXPORT) @@ -1071,9 +1057,9 @@ env_var_t env_get_string(const wcstring &key) env_universal_barrier(); } - const wchar_t *item = env_universal_get(key); + env_var_t item = env_universal_get(key); - if (!item || (wcscmp(item, ENV_NULL)==0)) + if (item.missing() || (wcscmp(item.c_str(), ENV_NULL)==0)) { return env_var_t::missing_var(); } @@ -1087,7 +1073,6 @@ env_var_t env_get_string(const wcstring &key) bool env_exist(const wchar_t *key, int mode) { env_node_t *env; - const wchar_t *item = NULL; CHECK(key, false); @@ -1151,9 +1136,7 @@ bool env_exist(const wchar_t *key, int mode) env_universal_barrier(); } - item = env_universal_get(key); - - if (item != NULL) + if (! env_universal_get(key).missing()) { if (mode & ENV_EXPORT) { @@ -1419,9 +1402,9 @@ static void update_export_array_if_necessary(bool recalc) for (i=0; i var_table_t; #endif diff --git a/env_universal.cpp b/env_universal.cpp index 72e9cc0e6..64f398327 100644 --- a/env_universal.cpp +++ b/env_universal.cpp @@ -38,6 +38,7 @@ #include "wutil.h" #include "env_universal_common.h" #include "env_universal.h" +#include "env.h" /** Maximum number of times to try to get a new fishd socket @@ -345,10 +346,10 @@ int env_universal_read_all() } } -const wchar_t *env_universal_get(const wcstring &name) +env_var_t env_universal_get(const wcstring &name) { if (!s_env_univeral_inited) - return NULL; + return env_var_t::missing_var(); return env_universal_common_get(name); } @@ -462,14 +463,15 @@ int env_universal_remove(const wchar_t *name) CHECK(name, 1); - res = !env_universal_common_get(name); + wcstring name_str = name; + res = env_universal_common_get(name_str).missing(); debug(3, L"env_universal_remove( \"%ls\" )", name); if (is_dead()) { - env_universal_common_remove(wcstring(name)); + env_universal_common_remove(name_str); } else { diff --git a/env_universal.h b/env_universal.h index 4f38fe796..a77efb1f5 100644 --- a/env_universal.h +++ b/env_universal.h @@ -8,6 +8,7 @@ #include #include "env_universal_common.h" +#include "env.h" /** Data about the universal variable server. @@ -29,7 +30,7 @@ void env_universal_destroy(); /** Get the value of a universal variable */ -const wchar_t *env_universal_get(const wcstring &name); +env_var_t env_universal_get(const wcstring &name); /** Get the export flag of the variable with the specified diff --git a/env_universal_common.cpp b/env_universal_common.cpp index 048396afa..5194e8ff1 100644 --- a/env_universal_common.cpp +++ b/env_universal_common.cpp @@ -86,28 +86,13 @@ */ #define ENV_UNIVERSAL_EOF 0x102 -/** - A variable entry. Stores the value of a variable and whether it - should be exported. Obviously, it needs to be allocated large - enough to fit the value string. -*/ -typedef struct var_uni_entry -{ - bool exportv; /**< Whether the variable should be exported */ - wcstring val; /**< The value of the variable */ - var_uni_entry():exportv(false), val() { } -} -var_uni_entry_t; - - static void parse_message(wchar_t *msg, connection_t *src); /** The table of all universal variables */ -typedef std::map env_var_table_t; -env_var_table_t env_universal_var; +static env_universal_t s_env_universal_var; /** Callback function, should be called on all events @@ -269,7 +254,7 @@ void read_message(connection_t *src) */ void env_universal_common_remove(const wcstring &name) { - env_universal_var.erase(name); + s_env_universal_var.remove(name); } /** @@ -291,10 +276,8 @@ void env_universal_common_set(const wchar_t *key, const wchar_t *val, bool expor { CHECK(key,); CHECK(val,); - - var_uni_entry_t &entry = env_universal_var[key]; - entry.exportv=exportv; - entry.val = val; + + s_env_universal_var.set(key, val, exportv); if (callback) { @@ -601,60 +584,26 @@ message_t *create_message(fish_message_type_t type, /** Put exported or unexported variables in a string list */ -void env_universal_common_get_names(wcstring_list_t &lst, - bool show_exported, - bool show_unexported) +void env_universal_common_get_names(wcstring_list_t &lst, bool show_exported, bool show_unexported) { - env_var_table_t::const_iterator iter; - for (iter = env_universal_var.begin(); iter != env_universal_var.end(); ++iter) - { - const wcstring &key = iter->first; - const var_uni_entry_t &e = iter->second; - if ((e.exportv && show_exported) || (! e.exportv && show_unexported)) - { - lst.push_back(key); - } - } - + wcstring_list_t names = s_env_universal_var.get_names(show_exported, show_unexported); + lst.insert(lst.end(), names.begin(), names.end()); } -const wchar_t *env_universal_common_get(const wcstring &name) +env_var_t env_universal_common_get(const wcstring &name) { - env_var_table_t::const_iterator result = env_universal_var.find(name); - if (result != env_universal_var.end()) - { - const var_uni_entry_t &e = result->second; - return const_cast(e.val.c_str()); - } - - return NULL; + return s_env_universal_var.get(name); } bool env_universal_common_get_export(const wcstring &name) { - env_var_table_t::const_iterator result = env_universal_var.find(name); - if (result != env_universal_var.end()) - { - const var_uni_entry_t &e = result->second; - return e.exportv; - } - return false; + return s_env_universal_var.get_export(name); } void enqueue_all(connection_t *c) { - env_var_table_t::const_iterator iter; - for (iter = env_universal_var.begin(); iter != env_universal_var.end(); ++iter) - { - const wcstring &key = iter->first; - const var_uni_entry_t &entry = iter->second; - - message_t *msg = create_message(entry.exportv ? SET_EXPORT : SET, key.c_str(), entry.val.c_str()); - msg->count=1; - c->unsent.push(msg); - } - + s_env_universal_var.enqueue_all(c); try_send_all(c); } @@ -679,3 +628,81 @@ void connection_destroy(connection_t *c) } } } + +env_universal_t::env_universal_t() +{ + VOMIT_ON_FAILURE(pthread_mutex_init(&lock, NULL)); +} + +env_universal_t::~env_universal_t() +{ + pthread_mutex_destroy(&lock); +} + +env_var_t env_universal_t::get(const wcstring &name) const +{ + env_var_t result = env_var_t::missing_var(); + var_table_t::const_iterator where = vars.find(name); + if (where != vars.end()) + { + result = where->second.val; + } + return result; +} + +bool env_universal_t::get_export(const wcstring &name) const +{ + bool result = false; + var_table_t::const_iterator where = vars.find(name); + if (where != vars.end()) + { + result = where->second.exportv; + } + return result; +} + +void env_universal_t::set(const wcstring &key, const wcstring &val, bool exportv) +{ + scoped_lock locker(lock); + var_entry_t *entry = &vars[key]; + entry->val = val; + entry->exportv = exportv; +} + +void env_universal_t::remove(const wcstring &key) +{ + scoped_lock locker(lock); + vars.erase(key); +} + +wcstring_list_t env_universal_t::get_names(bool show_exported, bool show_unexported) const +{ + wcstring_list_t result; + scoped_lock locker(lock); + var_table_t::const_iterator iter; + for (iter = vars.begin(); iter != vars.end(); ++iter) + { + const wcstring &key = iter->first; + const var_entry_t &e = iter->second; + if ((e.exportv && show_exported) || (! e.exportv && show_unexported)) + { + result.push_back(key); + } + } + return result; +} + +void env_universal_t::enqueue_all(connection_t *c) const +{ + scoped_lock locker(lock); + var_table_t::const_iterator iter; + for (iter = vars.begin(); iter != vars.end(); ++iter) + { + const wcstring &key = iter->first; + const var_entry_t &entry = iter->second; + + message_t *msg = create_message(entry.exportv ? SET_EXPORT : SET, key.c_str(), entry.val.c_str()); + msg->count=1; + c->unsent.push(msg); + } +} diff --git a/env_universal_common.h b/env_universal_common.h index 0a13a4187..100cbfc45 100644 --- a/env_universal_common.h +++ b/env_universal_common.h @@ -5,6 +5,7 @@ #include #include #include "util.h" +#include "env.h" /** The set command @@ -176,7 +177,7 @@ void env_universal_common_remove(const wcstring &key); This function operate agains the local copy of all universal variables, it does not communicate with any other process. */ -const wchar_t *env_universal_common_get(const wcstring &name); +env_var_t env_universal_common_get(const wcstring &name); /** Get the export flag of the variable with the specified @@ -199,4 +200,32 @@ void enqueue_all(connection_t *c); */ void connection_destroy(connection_t *c); +/** Class representing universal variables */ +class env_universal_t +{ + var_table_t vars; + mutable pthread_mutex_t lock; +public: + env_universal_t(); + ~env_universal_t(); + + /* Get the value of the variable with the specified name */ + env_var_t get(const wcstring &name) const; + + /* Returns whether the variable with the given name is exported, or false if it does not exist */ + bool get_export(const wcstring &name) const; + + /* Sets a variable */ + void set(const wcstring &key, const wcstring &val, bool exportv); + + /* Removes a variable */ + void remove(const wcstring &name); + + /* Gets variable names */ + wcstring_list_t get_names(bool show_exported, bool show_unexported) const; + + /* Writes variables to the connection */ + void enqueue_all(connection_t *c) const; +}; + #endif diff --git a/fishd.cpp b/fishd.cpp index 30ded3cdd..e20c8bb57 100644 --- a/fishd.cpp +++ b/fishd.cpp @@ -720,8 +720,7 @@ static env_var_t fishd_env_get(const char *key) else { const wcstring wkey = str2wcstring(key); - const wchar_t *tmp = env_universal_common_get(wkey); - return tmp ? env_var_t(tmp) : env_var_t::missing_var(); + return env_universal_common_get(wkey); } }