From fac2f27dd3d31d89b0f822b0e7d6042dfb2240cd Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Wed, 30 Apr 2014 10:17:36 -0700 Subject: [PATCH] Implement deletion of universal variables. Add tests for them. --- env_universal.cpp | 7 +----- env_universal_common.cpp | 46 ++++++++++++++++++++++++++++++++++------ env_universal_common.h | 1 + fish_tests.cpp | 28 ++++++++++++++++++------ 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/env_universal.cpp b/env_universal.cpp index 6387677d4..6d197bbb3 100644 --- a/env_universal.cpp +++ b/env_universal.cpp @@ -277,11 +277,6 @@ static void reconnect() } } -void env_universal_read_from_file() -{ - -} - void env_universal_init(wchar_t * p, wchar_t *u, void (*sf)(), @@ -291,8 +286,8 @@ void env_universal_init(wchar_t * p, { external_callback = cb; env_universal_common_init(&callback); - env_universal_read_from_file(); s_env_univeral_inited = true; + env_universal_common_sync(); } else { diff --git a/env_universal_common.cpp b/env_universal_common.cpp index 567bdeb44..2fab4efc4 100644 --- a/env_universal_common.cpp +++ b/env_universal_common.cpp @@ -647,6 +647,33 @@ void env_universal_t::enqueue_all(connection_t *c) const enqueue_all_internal(c); } +void env_universal_t::clear_values_for_unmodified_keys() +{ + // Little optimization + if (modified.empty()) + { + // Nothing modified, clear all values + vars.clear(); + return; + } + + var_table_t::iterator iter = vars.begin(); + while (iter != vars.end()) + { + const wcstring &key = iter->first; + if (modified.find(key) != modified.end()) + { + // It's modified, don't erase the value + ++iter; + } + else + { + // Unmodified, clear the value from the map + vars.erase(iter++); + } + } +} + void env_universal_t::load_from_fd(int fd, callback_data_list_t *callbacks) { ASSERT_IS_LOCKED(lock); @@ -655,6 +682,9 @@ void env_universal_t::load_from_fd(int fd, callback_data_list_t *callbacks) const file_id_t current_file = file_id_for_fd(fd); if (current_file != last_read_file) { + /* Erase all non-modified keys. Some of these may have been deleted in the file, and the only way we can tell is by their absence in the file. */ + this->clear_values_for_unmodified_keys(); + connection_t c(fd); /* Read from the file. Do not destroy the connection; the caller is responsible for closing the fd. */ this->read_message_internal(&c, callbacks); @@ -665,14 +695,18 @@ void env_universal_t::load_from_fd(int fd, callback_data_list_t *callbacks) bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t *callbacks) { ASSERT_IS_LOCKED(lock); - /* OK to not use CLO_EXEC here because fishd is single threaded */ bool result = false; - int fd = wopen_cloexec(path, O_RDONLY); - if (fd >= 0) + /* Skip the file if it has not changed. This is the primary mechanism by which we elide expensive re-syncing. */ + const file_id_t current_file = file_id_for_path(path); + if (current_file != last_read_file) { - this->load_from_fd(fd, callbacks); - close(fd); - result = true; + int fd = wopen_cloexec(path, O_RDONLY); + if (fd >= 0) + { + this->load_from_fd(fd, callbacks); + close(fd); + result = true; + } } return result; } diff --git a/env_universal_common.h b/env_universal_common.h index 769825ed6..a8c1ab116 100644 --- a/env_universal_common.h +++ b/env_universal_common.h @@ -227,6 +227,7 @@ class env_universal_t void set_internal(const wcstring &key, const wcstring &val, bool exportv, bool overwrite); void remove_internal(const wcstring &name, bool overwrite); + void clear_values_for_unmodified_keys(); /* Functions concerned with saving */ bool open_and_acquire_lock(const wcstring &path, int *out_fd); diff --git a/fish_tests.cpp b/fish_tests.cpp index 73a63a88b..c10a5ca81 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -2160,7 +2160,7 @@ static void test_input() } } -#define UVARS_PER_THREAD 8 +#define UVARS_PER_THREAD 16 static int test_universal_helper(int *x) { @@ -2176,8 +2176,14 @@ static int test_universal_helper(int *x) err(L"Failed to sync universal variables"); } fputc('.', stderr); - fflush(stderr); } + + /* Delete the first key */ + const wcstring key = format_string(L"key_%d_%d", *x, 0); + uvars.remove(key); + uvars.sync(NULL); + fputc('.', stderr); + return 0; } @@ -2204,11 +2210,21 @@ static void test_universal() for (int j=0; j < UVARS_PER_THREAD; j++) { const wcstring key = format_string(L"key_%d_%d", i, j); - const wcstring val = format_string(L"val_%d_%d", i, j); - const env_var_t var = uvars.get(key); - if (var != val) + env_var_t expected_val; + if (j == 0) { - err(L"Wrong value for key %ls: %ls vs %ls\n", key.c_str(), val.c_str(), var.missing() ? L"" : var.c_str()); + expected_val = env_var_t::missing_var(); + } + else + { + expected_val = format_string(L"val_%d_%d", i, j); + } + const env_var_t var = uvars.get(key); + if (var != expected_val) + { + const wchar_t *missing = L""; + + err(L"Wrong value for key %ls: expected %ls, got %ls\n", key.c_str(), expected_val.missing() ? missing : expected_val.c_str(), var.missing() ? missing : var.c_str()); } } }