Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2025-01-01 07:38:46 +00:00
Code Issues Projects Releases Packages Wiki Activity

Make history_filename return a maybe_t<wcstring>

Browse source 
This function can fail, so rather than forcing clients to check the return
value as empty, allow it to return none().
This commit is contained in:
ridiculousfish 2020-01-01 12:34:42 -08:00
parent efa9d5dd6a
commit 5aa22adccc
1 changed files with 38 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

75
src/history.cpp
View file

@ -106,6 +106,21 @@ class time_profiler_t {
} }
}; };
/// \return the path for the history file for the given \p session_id, or none() if it could not be
/// loaded. If suffix is provided, append that suffix to the path; this is used for temporary files.
static maybe_t<wcstring> history_filename(const wcstring &session_id, const wcstring &suffix = {}) {
if (session_id.empty()) return none();
wcstring result;
if (!path_get_data(result)) return none();
result.append(L"/");
result.append(session_id);
result.append(L"_history");
result.append(suffix);
return result;
}
/// Lock the history file. /// Lock the history file.
/// Returns true on success, false on failure. /// Returns true on success, false on failure.
static bool history_file_lock(int fd, int lock_type) { static bool history_file_lock(int fd, int lock_type) {
@ -153,8 +168,6 @@ class history_lru_cache_t : public lru_cache_t<history_lru_cache_t, history_item
} }
}; };
static wcstring history_filename(const wcstring &session_id, const wcstring &suffix);
/// We can merge two items if they are the same command. We use the more recent timestamp, more /// We can merge two items if they are the same command. We use the more recent timestamp, more
/// recent identifier, and the longer list of required paths. /// recent identifier, and the longer list of required paths.
bool history_item_t::merge(const history_item_t &item) { bool history_item_t::merge(const history_item_t &item) {
@ -554,9 +567,8 @@ void history_impl_t::load_old_if_needed() {
loaded_old = true; loaded_old = true;
time_profiler_t profiler("load_old"); //!OCLINT(side-effect) time_profiler_t profiler("load_old"); //!OCLINT(side-effect)
wcstring filename = history_filename(name, L""); if (maybe_t<wcstring> filename = history_filename(name)) {
if (!filename.empty()) { int fd = wopen_cloexec(*filename, O_RDONLY);
int fd = wopen_cloexec(filename, O_RDONLY);
if (fd >= 0) { if (fd >= 0) {
// Take a read lock to guard against someone else appending. This is released when the // Take a read lock to guard against someone else appending. This is released when the
// file is closed (below). We will read the file after releasing the lock, but that's // file is closed (below). We will read the file after releasing the lock, but that's
@ -624,19 +636,6 @@ const history_item_t &history_search_t::current_item() const {
const wcstring &history_search_t::current_string() const { return this->current_item().str(); } const wcstring &history_search_t::current_string() const { return this->current_item().str(); }
static wcstring history_filename(const wcstring &session_id, const wcstring &suffix) {
if (session_id.empty()) return L"";
wcstring result;
if (!path_get_data(result)) return L"";
result.append(L"/");
result.append(session_id);
result.append(L"_history");
result.append(suffix);
return result;
}
void history_impl_t::clear_file_state() { void history_impl_t::clear_file_state() {
// Erase everything we know about our file. // Erase everything we know about our file.
file_contents.reset(); file_contents.reset();
@ -748,16 +747,16 @@ bool history_impl_t::save_internal_via_rewrite() {
// We want to rewrite the file, while holding the lock for as briefly as possible // We want to rewrite the file, while holding the lock for as briefly as possible
// To do this, we speculatively write a file, and then lock and see if our original file changed // To do this, we speculatively write a file, and then lock and see if our original file changed
// Repeat until we succeed or give up // Repeat until we succeed or give up
const wcstring target_name = history_filename(name, wcstring()); const maybe_t<wcstring> target_name = history_filename(name);
const wcstring tmp_name_template = history_filename(name, L".XXXXXX"); const maybe_t<wcstring> tmp_name_template = history_filename(name, L".XXXXXX");
if (target_name.empty() || tmp_name_template.empty()) { if (!target_name.has_value() || !tmp_name_template.has_value()) {
return false; return false;
} }
// Make our temporary file // Make our temporary file
// Remember that we have to close this fd! // Remember that we have to close this fd!
wcstring tmp_name; wcstring tmp_name;
int tmp_fd = create_temporary_file(tmp_name_template, &tmp_name); int tmp_fd = create_temporary_file(*tmp_name_template, &tmp_name);
if (tmp_fd < 0) { if (tmp_fd < 0) {
return false; return false;
} }
@ -765,7 +764,7 @@ bool history_impl_t::save_internal_via_rewrite() {
bool done = false; bool done = false;
for (int i = 0; i < max_save_tries && !done; i++) { for (int i = 0; i < max_save_tries && !done; i++) {
// Open any target file, but do not lock it right away // Open any target file, but do not lock it right away
int target_fd_before = wopen_cloexec(target_name, O_RDONLY | O_CREAT, history_file_mode); int target_fd_before = wopen_cloexec(*target_name, O_RDONLY | O_CREAT, history_file_mode);
file_id_t orig_file_id = file_id_for_fd(target_fd_before); // possibly invalid file_id_t orig_file_id = file_id_for_fd(target_fd_before); // possibly invalid
bool wrote = this->rewrite_to_temporary_file(target_fd_before, tmp_fd); bool wrote = this->rewrite_to_temporary_file(target_fd_before, tmp_fd);
if (target_fd_before >= 0) { if (target_fd_before >= 0) {
@ -780,14 +779,14 @@ bool history_impl_t::save_internal_via_rewrite() {
// were rewriting it. Make an effort to take the lock before checking, to avoid racing. // were rewriting it. Make an effort to take the lock before checking, to avoid racing.
// If the open fails, then proceed; this may be because there is no current history // If the open fails, then proceed; this may be because there is no current history
file_id_t new_file_id = kInvalidFileID; file_id_t new_file_id = kInvalidFileID;
int target_fd_after = wopen_cloexec(target_name, O_RDONLY); int target_fd_after = wopen_cloexec(*target_name, O_RDONLY);
if (target_fd_after >= 0) { if (target_fd_after >= 0) {
// critical to take the lock before checking file IDs, // critical to take the lock before checking file IDs,
// and hold it until after we are done replacing // and hold it until after we are done replacing
// Also critical to check the file at the path, NOT based on our fd // Also critical to check the file at the path, NOT based on our fd
// It's only OK to replace the file while holding the lock // It's only OK to replace the file while holding the lock
history_file_lock(target_fd_after, LOCK_EX); history_file_lock(target_fd_after, LOCK_EX);
new_file_id = file_id_for_path(target_name); new_file_id = file_id_for_path(*target_name);
} }
bool can_replace_file = (new_file_id == orig_file_id || new_file_id == kInvalidFileID); bool can_replace_file = (new_file_id == orig_file_id || new_file_id == kInvalidFileID);
if (!can_replace_file) { if (!can_replace_file) {
@ -816,7 +815,7 @@ bool history_impl_t::save_internal_via_rewrite() {
} }
// Slide it into place // Slide it into place
if (wrename(tmp_name, target_name) == -1) { if (wrename(tmp_name, *target_name) == -1) {
debug(2, L"Error %d when renaming history file", errno); debug(2, L"Error %d when renaming history file", errno);
} }
@ -862,10 +861,11 @@ bool history_impl_t::save_internal_via_appending() {
bool file_changed = false; bool file_changed = false;
// Get the path to the real history file. // Get the path to the real history file.
wcstring history_path = history_filename(name, wcstring()); maybe_t<wcstring> maybe_history_path = history_filename(name);
if (history_path.empty()) { if (!maybe_history_path) {
return true; return true;
} }
wcstring history_path = maybe_history_path.acquire();
// We are going to open the file, lock it, append to it, and then close it // We are going to open the file, lock it, append to it, and then close it
// After locking it, we need to stat the file at the path; if there is a new file there, it // After locking it, we need to stat the file at the path; if there is a new file there, it
@ -964,7 +964,7 @@ void history_impl_t::save(bool vacuum) {
// Nothing to do if there's no new items. // Nothing to do if there's no new items.
if (first_unwritten_new_item_index >= new_items.size() && deleted_items.empty()) return; if (first_unwritten_new_item_index >= new_items.size() && deleted_items.empty()) return;
if (history_filename(name, L"").empty()) { if (!history_filename(name).has_value()) {
// We're in the "incognito" mode. Pretend we've saved the history. // We're in the "incognito" mode. Pretend we've saved the history.
this->first_unwritten_new_item_index = new_items.size(); this->first_unwritten_new_item_index = new_items.size();
this->deleted_items.clear(); this->deleted_items.clear();
@ -1030,8 +1030,9 @@ void history_impl_t::clear() {
deleted_items.clear(); deleted_items.clear();
first_unwritten_new_item_index = 0; first_unwritten_new_item_index = 0;
old_item_offsets.clear(); old_item_offsets.clear();
wcstring filename = history_filename(name, L""); if (maybe_t<wcstring> filename = history_filename(name)) {
if (!filename.empty()) wunlink(filename); wunlink(*filename);
}
this->clear_file_state(); this->clear_file_state();
} }
@ -1048,13 +1049,13 @@ bool history_impl_t::is_empty() {
} else { } else {
// If we have not loaded old items, don't actually load them (which may be expensive); just // If we have not loaded old items, don't actually load them (which may be expensive); just
// stat the file and see if it exists and is nonempty. // stat the file and see if it exists and is nonempty.
const wcstring where = history_filename(name, L""); const maybe_t<wcstring> where = history_filename(name);
if (where.empty()) { if (!where.has_value()) {
return true; return true;
} }
struct stat buf = {}; struct stat buf = {};
if (wstat(where, &buf) != 0) { if (wstat(*where, &buf) != 0) {
// Access failed, assume missing. // Access failed, assume missing.
empty = true; empty = true;
} else { } else {
@ -1069,8 +1070,8 @@ bool history_impl_t::is_empty() {
/// clearing ourselves, and copying the contents of the old history file to the new history file. /// clearing ourselves, and copying the contents of the old history file to the new history file.
/// The new contents will automatically be re-mapped later. /// The new contents will automatically be re-mapped later.
void history_impl_t::populate_from_config_path() { void history_impl_t::populate_from_config_path() {
wcstring new_file = history_filename(name, wcstring()); maybe_t<wcstring> new_file = history_filename(name);
if (new_file.empty()) { if (!new_file.has_value()) {
return; return;
} }
@ -1085,7 +1086,7 @@ void history_impl_t::populate_from_config_path() {
// destination file descriptor, since it destroys the name and the file. // destination file descriptor, since it destroys the name and the file.
this->clear(); this->clear();
int dst_fd = wopen_cloexec(new_file, O_WRONLY | O_CREAT, history_file_mode); int dst_fd = wopen_cloexec(*new_file, O_WRONLY | O_CREAT, history_file_mode);
char buf[BUFSIZ]; char buf[BUFSIZ];
ssize_t size; ssize_t size;
while ((size = read(src_fd, buf, BUFSIZ)) > 0) { while ((size = read(src_fd, buf, BUFSIZ)) > 0) {