From e07de09460a81fcf020ff989041ec2116992c02f Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 19 Jan 2013 13:16:21 -0800 Subject: [PATCH] Big cleanup of env_set. Changed var_table_t to use direct var_entry_t instead of pointers. Changed some ints to bools. --- env.cpp | 344 +++++++++++++++++---------------------- env.h | 4 +- env_universal.cpp | 10 +- env_universal.h | 8 +- env_universal_common.cpp | 24 +-- env_universal_common.h | 10 +- 6 files changed, 178 insertions(+), 222 deletions(-) diff --git a/env.cpp b/env.cpp index 4787827e9..2c0ceb3dd 100644 --- a/env.cpp +++ b/env.cpp @@ -97,7 +97,7 @@ struct var_entry_t var_entry_t() : exportv(false) { } }; -typedef std::map var_table_t; +typedef std::map var_table_t; bool g_log_forks = false; bool g_use_posix_spawn = false; //will usually be set to true @@ -122,7 +122,7 @@ struct env_node_t /** Does this node contain any variables which are exported to subshells */ - int exportv; + bool exportv; /** Pointer to next level @@ -130,7 +130,7 @@ struct env_node_t struct env_node_t *next; - env_node_t() : new_scope(0), exportv(0), next(NULL) { } + env_node_t() : new_scope(0), exportv(false), next(NULL) { } }; class variable_entry_t @@ -200,17 +200,6 @@ static void mark_changed_exported() */ static wcstring dyn_var; -/** - Variable used by env_get_names to communicate auxiliary information - to add_key_to_string_set -*/ -static int get_names_show_exported; -/** - Variable used by env_get_names to communicate auxiliary information - to add_key_to_string_set -*/ -static int get_names_show_unexported; - /** List of all locale variable names */ @@ -678,7 +667,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) long shlvl_i = wcstol(shlvl_str.c_str(), NULL, 10); if (shlvl_i >= 0) { - nshlvl_str = format_string(L"%ld", 1 + shlvl_i); + nshlvl_str = to_string(shlvl_i + 1); } } env_set(L"SHLVL", nshlvl_str.c_str(), ENV_GLOBAL | ENV_EXPORT); @@ -707,17 +696,15 @@ void env_destroy() env_read_only.clear(); env_electric.clear(); - var_table_t::iterator iter; for (iter = global->begin(); iter != global->end(); ++iter) { - var_entry_t *entry = iter->second; - if (entry->exportv) + const var_entry_t &entry = iter->second; + if (entry.exportv) { mark_changed_exported(); + break; } - - delete entry; } delete top; @@ -752,14 +739,12 @@ static env_node_t *env_get_node(const wcstring &key) int env_set(const wcstring &key, const wchar_t *val, int var_mode) { ASSERT_IS_MAIN_THREAD(); - env_node_t *node = NULL; bool has_changed_old = has_changed_exported; bool has_changed_new = false; - var_entry_t *e=0; int done=0; - + int is_universal = 0; - + if (val && contains(key, L"PWD", L"HOME")) { /* Canoncalize our path; if it changes, recurse and try again. */ @@ -770,204 +755,190 @@ int env_set(const wcstring &key, const wchar_t *val, int var_mode) return env_set(key, val_canonical.c_str(), var_mode); } } - + if ((var_mode & ENV_USER) && is_read_only(key)) { return ENV_PERM; } - + if (key == L"umask") { wchar_t *end; - + /* - Set the new umask - */ + Set the new umask + */ if (val && wcslen(val)) { errno=0; long mask = wcstol(val, &end, 8); - + if (!errno && (!*end) && (mask <= 0777) && (mask >= 0)) { umask(mask); } } - /* - Do not actually create a umask variable, on env_get, it will - be calculated dynamically - */ + /* Do not actually create a umask variable, on env_get, it will be calculated dynamically */ return 0; } - + /* - Zero element arrays are internaly not coded as null but as this - placeholder string - */ + Zero element arrays are internaly not coded as null but as this + placeholder string + */ if (!val) { val = ENV_NULL; } - + if (var_mode & ENV_UNIVERSAL) { - int exportv; - - if (!(var_mode & ENV_EXPORT) && - !(var_mode & ENV_UNEXPORT)) + bool exportv; + if (var_mode & ENV_EXPORT) { - exportv = env_universal_get_export(key); + // export + exportv = true; + } + else if (var_mode & ENV_UNEXPORT) + { + // unexport + exportv = false; } else { - exportv = (var_mode & ENV_EXPORT); + // not changing the export + exportv = env_universal_get_export(key); } - env_universal_set(key, val, exportv); is_universal = 1; - + } else { - - node = env_get_node(key); - if (node) + // Determine the node + + env_node_t *preexisting_node = env_get_node(key); + bool preexisting_entry_exportv = false; + if (preexisting_node != NULL) { - var_table_t::iterator result = node->env.find(key); - assert(result != node->env.end()); - e = result->second; - - if (e->exportv) + var_table_t::const_iterator result = preexisting_node->env.find(key); + assert(result != preexisting_node->env.end()); + const var_entry_t &entry = result->second; + if (entry.exportv) { + preexisting_entry_exportv = true; has_changed_new = true; } } - - if ((var_mode & ENV_LOCAL) || - (var_mode & ENV_GLOBAL)) + + env_node_t *node = NULL; + if (var_mode & ENV_GLOBAL) { - node = (var_mode & ENV_GLOBAL)?global_env:top; + node = global_env; + } + else if (var_mode & ENV_LOCAL) + { + node = top; + } + else if (preexisting_node != NULL) + { + node = preexisting_node; + + if ((var_mode & (ENV_EXPORT | ENV_UNEXPORT)) == 0) + { + // use existing entry's exportv + var_mode = preexisting_entry_exportv ? ENV_EXPORT : 0; + } } else { - if (node) + if (! get_proc_had_barrier()) { - if (!(var_mode & ENV_EXPORT) && - !(var_mode & ENV_UNEXPORT)) - { - var_mode = e->exportv?ENV_EXPORT:0; - } + set_proc_had_barrier(true); + env_universal_barrier(); } - else + + if (env_universal_get(key)) { - if (! get_proc_had_barrier()) + bool exportv; + if (var_mode & ENV_EXPORT) { - set_proc_had_barrier(true); - env_universal_barrier(); + exportv = true; } - - if (env_universal_get(key)) + else if (var_mode & ENV_UNEXPORT) { - int exportv; - - if (!(var_mode & ENV_EXPORT) && - !(var_mode & ENV_UNEXPORT)) - { - exportv = env_universal_get_export(key); - } - else - { - exportv = (var_mode & ENV_EXPORT); - } - - env_universal_set(key, val, exportv); - is_universal = 1; - - done = 1; - + exportv = false; } else { - /* - New variable with unspecified scope. The default - scope is the innermost scope that is shadowing, - which will be either the current function or the - global scope. - */ - node = top; - while (node->next && !node->new_scope) - { - node = node->next; - } + exportv = env_universal_get_export(key); + } + + env_universal_set(key, val, exportv); + is_universal = 1; + + done = 1; + + } + else + { + /* + New variable with unspecified scope. The default + scope is the innermost scope that is shadowing, + which will be either the current function or the + global scope. + */ + node = top; + while (node->next && !node->new_scope) + { + node = node->next; } } } - + if (!done) { - var_entry_t *old_entry = NULL; - var_table_t::iterator result = node->env.find(key); - if (result != node->env.end()) + // Set the entry in the node + // Note that operator[] accesses the existing entry, or creates a new one + var_entry_t &entry = node->env[key]; + if (entry.exportv) { - old_entry = result->second; - node->env.erase(result); + // this variable already existed, and was exported + has_changed_new = true; } - - var_entry_t *entry = NULL; - if (old_entry) + entry.val = val; + if (var_mode & ENV_EXPORT) { - entry = old_entry; - - if ((var_mode & ENV_EXPORT) || entry->exportv) - { - entry->exportv = !!(var_mode & ENV_EXPORT); - has_changed_new = true; - } + // the new variable is exported + entry.exportv = true; + node->exportv = true; + has_changed_new = true; } else { - entry = new var_entry_t; - - if (var_mode & ENV_EXPORT) - { - entry->exportv = 1; - has_changed_new = true; - } - else - { - entry->exportv = 0; - } - + entry.exportv = false; } - - entry->val = val; - node->env[key] = entry; - - if (entry->exportv) - { - node->exportv=1; - } - + if (has_changed_old || has_changed_new) mark_changed_exported(); } - + } - + if (!is_universal) { event_t ev = event_t::variable_event(key); ev.arguments.push_back(L"VARIABLE"); ev.arguments.push_back(L"SET"); ev.arguments.push_back(key); - + // debug( 1, L"env_set: fire events on variable %ls", key ); event_fire(&ev); // debug( 1, L"env_set: return from event firing" ); } - + react_to_variable_change(key); - + return 0; } @@ -978,33 +949,27 @@ int env_set(const wcstring &key, const wchar_t *val, int var_mode) \return zero if the variable was not found, non-zero otherwise */ -static int try_remove(env_node_t *n, - const wchar_t *key, - int var_mode) +static bool try_remove(env_node_t *n, const wchar_t *key, int var_mode) { - if (n == 0) + if (n == NULL) { - return 0; + return false; } var_table_t::iterator result = n->env.find(key); if (result != n->env.end()) { - var_entry_t *v = result->second; - - if (v->exportv) + if (result->second.exportv) { mark_changed_exported(); } - n->env.erase(result); - delete v; - return 1; + return true; } if (var_mode & ENV_LOCAL) { - return 0; + return false; } if (n->new_scope) @@ -1116,7 +1081,6 @@ env_var_t env_get_string(const wcstring &key) /* Lock around a local region */ scoped_lock lock(env_lock); - var_entry_t *res = NULL; env_node_t *env = top; wcstring result; @@ -1124,18 +1088,15 @@ env_var_t env_get_string(const wcstring &key) { var_table_t::iterator result = env->env.find(key); if (result != env->env.end()) - res = result->second; - - - if (res != NULL) { - if (res->val == ENV_NULL) + const var_entry_t &res = result->second; + if (res.val == ENV_NULL) { return env_var_t::missing_var(); } else { - return res->val; + return res.val; } } @@ -1171,13 +1132,12 @@ env_var_t env_get_string(const wcstring &key) } } -int env_exist(const wchar_t *key, int mode) +bool env_exist(const wchar_t *key, int mode) { - var_entry_t *res; env_node_t *env; wchar_t *item=0; - CHECK(key, 0); + CHECK(key, false); /* Read only variables all exist, and they are all global. A local @@ -1190,13 +1150,13 @@ int env_exist(const wchar_t *key, int mode) //Such variables are never exported if (mode & ENV_EXPORT) { - return 0; + return false; } else if (mode & ENV_UNEXPORT) { - return 1; + return true; } - return 1; + return true; } } @@ -1210,18 +1170,18 @@ int env_exist(const wchar_t *key, int mode) if (result != env->env.end()) { - res = result->second; + const var_entry_t &res = result->second; if (mode & ENV_EXPORT) { - return res->exportv == 1; + return res.exportv; } else if (mode & ENV_UNEXPORT) { - return res->exportv == 0; + return ! res.exportv; } - return 1; + return true; } if (mode & ENV_LOCAL) @@ -1330,12 +1290,12 @@ void env_pop() var_table_t::iterator iter; for (iter = killme->env.begin(); iter != killme->env.end(); ++iter) { - var_entry_t *entry = iter->second; - if (entry->exportv) + const var_entry_t &entry = iter->second; + if (entry.exportv) { mark_changed_exported(); + break; } - delete entry; } delete killme; @@ -1355,18 +1315,18 @@ void env_pop() /** Function used with to insert keys of one table into a set::set */ -static void add_key_to_string_set(const var_table_t &envs, std::set &strSet) +static void add_key_to_string_set(const var_table_t &envs, std::set *str_set, bool show_exported, bool show_unexported) { var_table_t::const_iterator iter; for (iter = envs.begin(); iter != envs.end(); ++iter) { - var_entry_t *e = iter->second; + const var_entry_t &e = iter->second; - if ((e->exportv && get_names_show_exported) || - (!e->exportv && get_names_show_unexported)) + if ((e.exportv && show_exported) || + (!e.exportv && show_unexported)) { - /*Insert Key*/ - strSet.insert(iter->first); + /* Insert this key */ + str_set->insert(iter->first); } } @@ -1383,12 +1343,8 @@ wcstring_list_t env_get_names(int flags) int show_universal = flags & ENV_UNIVERSAL; env_node_t *n=top; - - - get_names_show_exported = - (flags & ENV_EXPORT) || !(flags & ENV_UNEXPORT); - get_names_show_unexported = - (flags & ENV_UNEXPORT) || !(flags & ENV_EXPORT); + const bool show_exported = (flags & ENV_EXPORT) || !(flags & ENV_UNEXPORT); + const bool show_unexported = (flags & ENV_UNEXPORT) || !(flags & ENV_EXPORT); if (!show_local && !show_global && !show_universal) { @@ -1402,7 +1358,7 @@ wcstring_list_t env_get_names(int flags) if (n == global_env) break; - add_key_to_string_set(n->env, names); + add_key_to_string_set(n->env, &names, show_exported, show_unexported); if (n->new_scope) break; else @@ -1413,13 +1369,13 @@ wcstring_list_t env_get_names(int flags) if (show_global) { - add_key_to_string_set(global_env->env, names); - if (get_names_show_unexported) + add_key_to_string_set(global_env->env, &names, show_exported, show_unexported); + if (show_unexported) { result.insert(result.end(), env_electric.begin(), env_electric.end()); } - if (get_names_show_exported) + if (show_exported) { result.push_back(L"COLUMNS"); result.push_back(L"LINES"); @@ -1432,8 +1388,8 @@ wcstring_list_t env_get_names(int flags) wcstring_list_t uni_list; env_universal_get_names2(uni_list, - get_names_show_exported, - get_names_show_unexported); + show_exported, + show_unexported); names.insert(uni_list.begin(), uni_list.end()); } @@ -1459,11 +1415,11 @@ static void get_exported(const env_node_t *n, std::map &h) for (iter = n->env.begin(); iter != n->env.end(); ++iter) { const wcstring &key = iter->first; - var_entry_t *val_entry = iter->second; - if (val_entry->exportv && (val_entry->val != ENV_NULL)) + const var_entry_t &val_entry = iter->second; + if (val_entry.exportv && (val_entry.val != ENV_NULL)) { // Don't use std::map::insert here, since we need to overwrite existing values from previous scopes - h[key] = val_entry->val; + h[key] = val_entry.val; } } } diff --git a/env.h b/env.h index 543e9c7e6..e8d2fa048 100644 --- a/env.h +++ b/env.h @@ -146,13 +146,13 @@ class env_var_t : public wcstring env_var_t env_get_string(const wcstring &key); /** - Returns 1 if the specified key exists. This can't be reliably done + Returns true if the specified key exists. This can't be reliably done using env_get, since env_get returns null for 0-element arrays \param key The name of the variable to remove \param mode the scope to search in. All scopes are searched if unset */ -int env_exist(const wchar_t *key, int mode); +bool env_exist(const wchar_t *key, int mode); /** Remove environemnt variable diff --git a/env_universal.cpp b/env_universal.cpp index c9dcc350b..2ef1c7c56 100644 --- a/env_universal.cpp +++ b/env_universal.cpp @@ -350,10 +350,10 @@ wchar_t *env_universal_get(const wcstring &name) return env_universal_common_get(name); } -int env_universal_get_export(const wcstring &name) +bool env_universal_get_export(const wcstring &name) { if (!init) - return 0; + return false; return env_universal_common_get_export(name); } @@ -421,7 +421,7 @@ void env_universal_barrier() } -void env_universal_set(const wcstring &name, const wcstring &value, int exportv) +void env_universal_set(const wcstring &name, const wcstring &value, bool exportv) { message_t *msg; @@ -483,8 +483,8 @@ int env_universal_remove(const wchar_t *name) } void env_universal_get_names2(wcstring_list_t &lst, - int show_exported, - int show_unexported) + bool show_exported, + bool show_unexported) { if (!init) return; diff --git a/env_universal.h b/env_universal.h index 3e91bfdb8..3637ba675 100644 --- a/env_universal.h +++ b/env_universal.h @@ -35,12 +35,12 @@ wchar_t *env_universal_get(const wcstring &name); Get the export flag of the variable with the specified name. Returns 0 if the variable doesn't exist. */ -int env_universal_get_export(const wcstring &name); +bool env_universal_get_export(const wcstring &name); /** Set the value of a universal variable */ -void env_universal_set(const wcstring &name, const wcstring &val, int exportv); +void env_universal_set(const wcstring &name, const wcstring &val, bool exportv); /** Erase a universal variable @@ -61,8 +61,8 @@ int env_universal_read_all(); \param show_unexported whether unexported variables should be shown */ void env_universal_get_names2(wcstring_list_t &list, - int show_exported, - int show_unexported); + bool show_exported, + bool show_unexported); /** Synchronize with fishd diff --git a/env_universal_common.cpp b/env_universal_common.cpp index 4f419bfeb..c7fc2cb37 100644 --- a/env_universal_common.cpp +++ b/env_universal_common.cpp @@ -93,9 +93,9 @@ */ typedef struct var_uni_entry { - int exportv; /**< Whether the variable should be exported */ + bool exportv; /**< Whether the variable should be exported */ wcstring val; /**< The value of the variable */ - var_uni_entry():exportv(0), val() { } + var_uni_entry():exportv(false), val() { } } var_uni_entry_t; @@ -562,19 +562,19 @@ void env_universal_common_remove(const wcstring &name) /** Test if the message msg contains the command cmd */ -static int match(const wchar_t *msg, const wchar_t *cmd) +static bool match(const wchar_t *msg, const wchar_t *cmd) { size_t len = wcslen(cmd); if (wcsncasecmp(msg, cmd, len) != 0) - return 0; + return false; if (msg[len] && msg[len]!= L' ' && msg[len] != L'\t') - return 0; + return false; - return 1; + return true; } -void env_universal_common_set(const wchar_t *key, const wchar_t *val, int exportv) +void env_universal_common_set(const wchar_t *key, const wchar_t *val, bool exportv) { var_uni_entry_t *entry; @@ -609,7 +609,7 @@ static void parse_message(wchar_t *msg, if (match(msg, SET_STR) || match(msg, SET_EXPORT_STR)) { wchar_t *name, *tmp; - int exportv = match(msg, SET_EXPORT_STR); + bool exportv = match(msg, SET_EXPORT_STR); name = msg+(exportv?wcslen(SET_EXPORT_STR):wcslen(SET_STR)); while (wcschr(L"\t ", *name)) @@ -903,8 +903,8 @@ 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, - int show_exported, - int show_unexported) + 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) @@ -936,7 +936,7 @@ wchar_t *env_universal_common_get(const wcstring &name) return 0; } -int env_universal_common_get_export(const wcstring &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()) @@ -945,7 +945,7 @@ int env_universal_common_get_export(const wcstring &name) if (e != NULL) return e->exportv; } - return 0; + return false; } void enqueue_all(connection_t *c) diff --git a/env_universal_common.h b/env_universal_common.h index 1f90b6bb5..4625725f9 100644 --- a/env_universal_common.h +++ b/env_universal_common.h @@ -151,8 +151,8 @@ void env_universal_common_destroy(); variables, it does not communicate with any other process. */ void env_universal_common_get_names(wcstring_list_t &lst, - int show_exported, - int show_unexported); + bool show_exported, + bool show_unexported); /** Perform the specified variable assignment. @@ -163,7 +163,7 @@ void env_universal_common_get_names(wcstring_list_t &lst, Do not call this function. Create a message to do it. This function is only to be used when fishd is dead. */ -void env_universal_common_set(const wchar_t *key, const wchar_t *val, int exportv); +void env_universal_common_set(const wchar_t *key, const wchar_t *val, bool exportv); /** Remove the specified variable. @@ -186,12 +186,12 @@ wchar_t *env_universal_common_get(const wcstring &name); /** Get the export flag of the variable with the specified - name. Returns 0 if the variable doesn't exist. + name. Returns false if the variable doesn't exist. This function operate agains the local copy of all universal variables, it does not communicate with any other process. */ -int env_universal_common_get_export(const wcstring &name); +bool env_universal_common_get_export(const wcstring &name); /** Add messages about all existing variables to the specified connection