diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 1790a990b..c8c51e918 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -248,7 +248,7 @@ static wcstring encode_serialized(const wcstring_list_t &vals) { return join_strings(vals, UVAR_ARRAY_SEP); } -env_universal_t::env_universal_t(wcstring path) : explicit_vars_path(std::move(path)) {} +env_universal_t::env_universal_t(wcstring path) : narrow_vars_path(wcs2string(path)), explicit_vars_path(std::move(path)) {} maybe_t env_universal_t::get(const wcstring &name) const { var_table_t::const_iterator where = vars.find(name); @@ -401,7 +401,7 @@ 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) { +bool env_universal_t::load_from_path(const std::string &path, callback_data_list_t &callbacks) { ASSERT_IS_LOCKED(lock); // Check to see if the file is unchanged. We do this again in load_from_fd, but this avoids @@ -412,7 +412,7 @@ bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t } bool result = false; - int fd = wopen_cloexec(path, O_RDONLY); + int fd = open_cloexec(path, O_RDONLY); if (fd >= 0) { debug(5, L"universal log reading from file"); this->load_from_fd(fd, callbacks); @@ -422,6 +422,10 @@ bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t return result; } +bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t &callbacks) { + std::string tmp = wcs2string(path); + return load_from_path(tmp, callbacks); +} /// Serialize the contents to a string. std::string env_universal_t::serialize_with_vars(const var_table_t &vars) { std::string storage; @@ -473,7 +477,7 @@ bool env_universal_t::move_new_vars_file_into_place(const wcstring &src, const w bool env_universal_t::initialize(callback_data_list_t &callbacks) { scoped_lock locker(lock); if (!explicit_vars_path.empty()) { - return load_from_path(explicit_vars_path, callbacks); + return load_from_path(narrow_vars_path, callbacks); } // Get the variables path; if there is none (e.g. HOME is bogus) it's hopeless. @@ -499,6 +503,11 @@ bool env_universal_t::initialize(callback_data_list_t &callbacks) { } } } + // If we succeeded, we store the *default path*, not the legacy one. + if (success) { + explicit_vars_path = *vars_path; + narrow_vars_path = wcs2string(*vars_path); + } return success; } @@ -554,7 +563,7 @@ static bool lock_uvar_file(int fd) { return check_duration(start_time); } -bool env_universal_t::open_and_acquire_lock(const wcstring &path, int *out_fd) { +bool env_universal_t::open_and_acquire_lock(const std::string &path, int *out_fd) { // Attempt to open the file for reading at the given path, atomically acquiring a lock. On BSD, // we can use O_EXLOCK. On Linux, we open the file, take a lock, and then compare fstat() to // stat(); if they match, it means that the file was not replaced before we acquired the lock. @@ -575,7 +584,7 @@ bool env_universal_t::open_and_acquire_lock(const wcstring &path, int *out_fd) { int fd = -1; while (fd == -1) { double start_time = timef(); - fd = wopen_cloexec(path, flags, 0644); + fd = open_cloexec(path, flags, 0644); if (fd == -1) { if (errno == EINTR) continue; // signaled; try again #ifdef O_EXLOCK @@ -652,25 +661,28 @@ bool env_universal_t::sync(callback_data_list_t &callbacks) { // instances of fish will not be able to obtain it. This seems to be a greater risk than that of // data loss on lockless NFS. Users who put their home directory on lockless NFS are playing // with fire anyways. - wcstring vars_path = explicit_vars_path; - if (vars_path.empty()) { - if (auto default_path = default_vars_path()) { - vars_path = default_path.acquire(); + if (explicit_vars_path.empty()) { + // Initialize the vars path from the default one. + // In some cases we don't "initialize()" before, so we're doing that now. + // This should only happen once. + // FIXME: Why don't we initialize()? + auto def_vars_path = default_vars_path(); + if (!def_vars_path) { + debug(2, L"No universal variable path available"); + return false; } - } - if (vars_path.empty()) { - debug(2, L"No universal variable path available"); - return false; + explicit_vars_path = *def_vars_path; + narrow_vars_path = wcs2string(explicit_vars_path); } // If we have no changes, just load. if (modified.empty()) { - this->load_from_path(vars_path, callbacks); + this->load_from_path(narrow_path, callbacks); debug(5, L"universal log no modifications"); return false; } - const wcstring directory = wdirname(vars_path); + const wcstring directory = wdirname(explicit_vars_path); bool success = true; int vars_fd = -1; @@ -678,7 +690,7 @@ bool env_universal_t::sync(callback_data_list_t &callbacks) { // Open the file. if (success) { - success = this->open_and_acquire_lock(vars_path, &vars_fd); + success = this->open_and_acquire_lock(narrow_vars_path, &vars_fd); if (!success) debug(5, L"universal log open_and_acquire_lock() failed"); } diff --git a/src/env_universal_common.h b/src/env_universal_common.h index 3a8daf6c9..61d85aa7e 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -45,14 +45,17 @@ class env_universal_t { // vars indicates a deleted value. std::unordered_set modified; + std::string narrow_vars_path; // Path that we save to. If empty, use the default. - const wcstring explicit_vars_path; + wcstring explicit_vars_path; + // Whether it's OK to save. This may be set to false if we discover that a future version of // fish wrote the uvars contents. bool ok_to_save{true}; mutable std::mutex lock; + bool load_from_path(const std::string &path, callback_data_list_t &callbacks); bool load_from_path(const wcstring &path, callback_data_list_t &callbacks); void load_from_fd(int fd, callback_data_list_t &callbacks); @@ -60,7 +63,7 @@ class env_universal_t { bool remove_internal(const wcstring &name); // Functions concerned with saving. - bool open_and_acquire_lock(const wcstring &path, int *out_fd); + bool open_and_acquire_lock(const std::string &path, int *out_fd); bool open_temporary_file(const wcstring &directory, wcstring *out_path, int *out_fd); bool write_to_fd(int fd, const wcstring &path); bool move_new_vars_file_into_place(const wcstring &src, const wcstring &dst);