diff --git a/src/history.cpp b/src/history.cpp index 7c33166ef..82c894aaa 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -939,43 +939,45 @@ void history_t::populate_from_mmap(void) { /// if successful. Returns the mapped memory region by reference. bool history_t::map_file(const wcstring &name, const char **out_map_start, size_t *out_map_len, file_id_t *file_id) { - bool result = false; wcstring filename = history_filename(name, L""); - if (!filename.empty()) { - int fd = wopen_cloexec(filename, O_RDONLY); - if (fd >= 0) { - // Get the file ID if requested. - if (file_id != NULL) *file_id = file_id_for_fd(fd); + if (filename.empty()) { + return false; + } - // 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 - // not a problem, because we never modify already written data. In short, the purpose of - // this lock is to ensure we don't see the file size change mid-update. - // - // We may fail to lock (e.g. on lockless NFS - see - // https://github.com/fish-shell/fish-shell/issues/685 ). In that case, we proceed as - // if it did not fail. The risk is that we may get an incomplete history item; this - // is unlikely because we only treat an item as valid if it has a terminating - // newline. - // - // Simulate a failing lock in chaos_mode. - if (!chaos_mode) history_file_lock(fd, F_RDLCK); - off_t len = lseek(fd, 0, SEEK_END); - if (len != (off_t)-1) { - size_t mmap_length = (size_t)len; - if (lseek(fd, 0, SEEK_SET) == 0) { - char *mmap_start; - if ((mmap_start = (char *)mmap(0, mmap_length, PROT_READ, MAP_PRIVATE, fd, - 0)) != MAP_FAILED) { - result = true; - *out_map_start = mmap_start; - *out_map_len = mmap_length; - } - } + int fd = wopen_cloexec(filename, O_RDONLY); + if (fd == -1) { + return false; + } + + bool result = false; + // Get the file ID if requested. + if (file_id != NULL) *file_id = file_id_for_fd(fd); + + // 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 not a + // problem, because we never modify already written data. In short, the purpose of this lock + // is to ensure we don't see the file size change mid-update. + // + // We may fail to lock (e.g. on lockless NFS - see issue #685. In that case, we proceed as + // if it did not fail. The risk is that we may get an incomplete history item; this is + // unlikely because we only treat an item as valid if it has a terminating newline. + // + // Simulate a failing lock in chaos_mode. + if (!chaos_mode) history_file_lock(fd, F_RDLCK); + off_t len = lseek(fd, 0, SEEK_END); + if (len != (off_t)-1) { + size_t mmap_length = (size_t)len; + if (lseek(fd, 0, SEEK_SET) == 0) { + char *mmap_start; + if ((mmap_start = (char *)mmap(0, mmap_length, PROT_READ, MAP_PRIVATE, fd, + 0)) != MAP_FAILED) { + result = true; + *out_map_start = mmap_start; + *out_map_len = mmap_length; } - close(fd); } } + close(fd); return result; }