From 61887c061b09c1ec7ff9b7643cfab04635276c0e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 26 Jan 2017 11:06:03 -0800 Subject: [PATCH] Migrate responsibility for node creation into var_stack_t --- src/env.cpp | 106 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 39 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index 493ec5149..cad768a6d 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -60,8 +60,14 @@ bool g_use_posix_spawn = false; // will usually be set to true /// Does the terminal have the "eat_newline_glitch". bool term_has_xn = false; -/// Struct representing one level in the function variable stack. -struct env_node_t { +// Struct representing one level in the function variable stack. +// Only our variable stack should create these +class env_node_t { + friend struct var_stack_t; + env_node_t(bool is_new_scope) : new_scope(is_new_scope) {} + +public: + /// Variable table. var_table_t env; /// Does this node imply a new variable scope? If yes, all non-global variables below this one @@ -69,11 +75,9 @@ struct env_node_t { /// will explode. bool new_scope; /// Does this node contain any variables which are exported to subshells. - bool exportv; + bool exportv = false; /// Pointer to next level. - struct env_node_t *next; - - env_node_t() : new_scope(false), exportv(false), next(NULL) {} + env_node_t *next = NULL; /// Returns a pointer to the given entry if present, or NULL. const var_entry_t *find_entry(const wcstring &key); @@ -90,6 +94,9 @@ class variable_entry_t { static pthread_mutex_t env_lock = PTHREAD_MUTEX_INITIALIZER; +static void mark_changed_exported(); +static int local_scope_exports(env_node_t *n); + // A class wrapping up a variable stack // Currently there is only one variable stack in fish, // but we can imagine having separate (linked) stacks @@ -100,8 +107,37 @@ struct var_stack_t { // Bottom node on the function stack. env_node_t *global_env = NULL; + + var_stack_t() { + this->top = new env_node_t(false); + this->global_env = this->top; + } + + ~var_stack_t() { + while (this->top) { + env_node_t *next = this->top->next; + delete this->top; + this->top = next; + } + } + + // Pushes a new node onto our stack + // Optionally creates a new scope for the node + void push(bool new_scope) { + env_node_t *node = new env_node_t(new_scope); + node->next = this->top; + this->top = node; + if (new_scope && local_scope_exports(this->top)) { + mark_changed_exported(); + } + } }; -static var_stack_t vars_stack; + +// Get the global variable stack +static var_stack_t &vars_stack() { + static var_stack_t global_stack; + return global_stack; +} /// Universal variables global instance. Initialized in env_init. static env_universal_t *s_universal_variables = NULL; @@ -157,10 +193,10 @@ const var_entry_t *env_node_t::find_entry(const wcstring &key) { return result; } -env_node_t *env_node_t::next_scope_to_search() { return this->new_scope ? vars_stack.global_env : this->next; } +env_node_t *env_node_t::next_scope_to_search() { return this->new_scope ? vars_stack().global_env : this->next; } const env_node_t *env_node_t::next_scope_to_search() const { - return this->new_scope ? vars_stack.global_env : this->next; + return this->new_scope ? vars_stack().global_env : this->next; } /// Return the current umask value. @@ -428,9 +464,6 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { env_electric.insert(L"status"); env_electric.insert(L"umask"); - vars_stack.top = new env_node_t; - vars_stack.global_env = vars_stack.top; - // Now the environment variable handling is set up, the next step is to insert valid data. // Import environment variables. Walk backwards so that the first one out of any duplicates wins @@ -534,7 +567,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { /// Search all visible scopes in order for the specified key. Return the first scope in which it was /// found. static env_node_t *env_get_node(const wcstring &key) { - env_node_t *env = vars_stack.top; + env_node_t *env = vars_stack().top; while (env != NULL) { if (env->find_entry(key) != NULL) { break; @@ -645,9 +678,9 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode) env_node_t *node = NULL; if (var_mode & ENV_GLOBAL) { - node = vars_stack.global_env; + node = vars_stack().global_env; } else if (var_mode & ENV_LOCAL) { - node = vars_stack.top; + node = vars_stack().top; } else if (preexisting_node != NULL) { node = preexisting_node; if ((var_mode & (ENV_EXPORT | ENV_UNEXPORT)) == 0) { @@ -679,7 +712,7 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode) } 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 = vars_stack.top; + node = vars_stack().top; while (node->next && !node->new_scope) { node = node->next; } @@ -744,7 +777,7 @@ static bool try_remove(env_node_t *n, const wchar_t *key, int var_mode) { } if (n->new_scope) { - return try_remove(vars_stack.global_env, key, var_mode); + return try_remove(vars_stack().global_env, key, var_mode); } return try_remove(n->next, key, var_mode); } @@ -758,11 +791,11 @@ int env_remove(const wcstring &key, int var_mode) { return 2; } - first_node = vars_stack.top; + first_node = vars_stack().top; if (!(var_mode & ENV_UNIVERSAL)) { if (var_mode & ENV_GLOBAL) { - first_node = vars_stack.global_env; + first_node = vars_stack().global_env; } if (try_remove(first_node, key.c_str(), var_mode)) { @@ -841,7 +874,7 @@ env_var_t env_get_string(const wcstring &key, env_mode_flags_t mode) { /* Lock around a local region */ scoped_lock locker(env_lock); - env_node_t *env = search_local ? vars_stack.top : vars_stack.global_env; + env_node_t *env = search_local ? vars_stack().top : vars_stack().global_env; while (env != NULL) { const var_entry_t *entry = env->find_entry(key); @@ -853,8 +886,8 @@ env_var_t env_get_string(const wcstring &key, env_mode_flags_t mode) { } if (has_scope) { - if (!search_global || env == vars_stack.global_env) break; - env = vars_stack.global_env; + if (!search_global || env == vars_stack().global_env) break; + env = vars_stack().global_env; } else { env = env->next_scope_to_search(); } @@ -902,9 +935,9 @@ bool env_exist(const wchar_t *key, env_mode_flags_t mode) { } if (test_local || test_global) { - const env_node_t *env = test_local ? vars_stack.top : vars_stack.global_env; + const env_node_t *env = test_local ? vars_stack().top : vars_stack().global_env; while (env != NULL) { - if (env == vars_stack.global_env && !test_global) { + if (env == vars_stack().global_env && !test_global) { break; } @@ -934,7 +967,7 @@ bool env_exist(const wchar_t *key, env_mode_flags_t mode) { /// Returns true if the specified scope or any non-shadowed non-global subscopes contain an exported /// variable. static int local_scope_exports(env_node_t *n) { - if (n == vars_stack.global_env) return 0; + if (n == vars_stack().global_env) return 0; if (n->exportv) return 1; @@ -944,19 +977,14 @@ static int local_scope_exports(env_node_t *n) { } void env_push(bool new_scope) { - env_node_t *node = new env_node_t; - node->next = vars_stack.top; - node->new_scope = new_scope; - - if (new_scope && local_scope_exports(vars_stack.top)) mark_changed_exported(); - vars_stack.top = node; + vars_stack().push(new_scope); } void env_pop() { - if (vars_stack.top != vars_stack.global_env) { + if (vars_stack().top != vars_stack().global_env) { int i; const wchar_t *locale_changed = NULL; - env_node_t *killme = vars_stack.top; + env_node_t *killme = vars_stack().top; for (i = 0; locale_variable[i]; i++) { var_table_t::iterator result = killme->env.find(locale_variable[i]); @@ -970,7 +998,7 @@ void env_pop() { if (killme->exportv || local_scope_exports(killme->next)) mark_changed_exported(); } - vars_stack.top = vars_stack.top->next; + vars_stack().top = vars_stack().top->next; var_table_t::iterator iter; for (iter = killme->env.begin(); iter != killme->env.end(); ++iter) { @@ -1014,7 +1042,7 @@ wcstring_list_t env_get_names(int flags) { int show_global = flags & ENV_GLOBAL; int show_universal = flags & ENV_UNIVERSAL; - env_node_t *n = vars_stack.top; + env_node_t *n = vars_stack().top; const bool show_exported = (flags & ENV_EXPORT) || !(flags & ENV_UNEXPORT); const bool show_unexported = (flags & ENV_UNEXPORT) || !(flags & ENV_EXPORT); @@ -1024,7 +1052,7 @@ wcstring_list_t env_get_names(int flags) { if (show_local) { while (n) { - if (n == vars_stack.global_env) break; + if (n == vars_stack().global_env) break; add_key_to_string_set(n->env, &names, show_exported, show_unexported); if (n->new_scope) @@ -1035,7 +1063,7 @@ wcstring_list_t env_get_names(int flags) { } if (show_global) { - add_key_to_string_set(vars_stack.global_env->env, &names, show_exported, show_unexported); + add_key_to_string_set(vars_stack().global_env->env, &names, show_exported, show_unexported); if (show_unexported) { result.insert(result.end(), env_electric.begin(), env_electric.end()); } @@ -1055,7 +1083,7 @@ static void get_exported(const env_node_t *n, std::map *h) { if (!n) return; if (n->new_scope) - get_exported(vars_stack.global_env, h); + get_exported(vars_stack().global_env, h); else get_exported(n->next, h); @@ -1116,7 +1144,7 @@ static void update_export_array_if_necessary(bool recalc) { debug(4, L"env_export_arr() recalc"); - get_exported(vars_stack.top, &vals); + get_exported(vars_stack().top, &vals); if (uvars()) { const wcstring_list_t uni = uvars()->get_names(true, false);