github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2024-11-23 03:44:04 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Change wopen_cloexec() to return File

Browse Source
This commit is contained in:
Mahmoud Al-Qudsi 2024-03-23 01:34:23 -05:00
parent 8d9d4ce1f9
commit 0ca199ef98
9 changed files with 44 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/autoload.rs
View File

@ -338,13 +338,12 @@ fn test_autoload() {
use std::fs::File;
use std::io::Write;
let fd = wopen_cloexec(
let mut file = wopen_cloexec(
path,
OFlag::O_RDWR | OFlag::O_CREAT,
Mode::from_bits_truncate(0o666),
)
.unwrap();
let mut file = File::from(fd);
file.write_all(b"Hello").unwrap();
}

10
src/builtins/source.rs
View File

@ -28,8 +28,8 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O
return STATUS_CMD_OK;
}
// If we open a file, this ensures we close it.
let opened_fd;
// If we open a file, this ensures it stays open until the end of scope.
let opened_file;
// The fd that we read from, either from opened_fd or stdin.
let fd;
@ -52,8 +52,8 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O
fd = streams.stdin_fd;
} else {
match wopen_cloexec(args[optind], OFlag::O_RDONLY, Mode::empty()) {
Ok(fd) => {
opened_fd = fd;
Ok(file) => {
opened_file = file;
}
Err(_) => {
let esc = escape(args[optind]);
@ -67,7 +67,7 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O
}
};
fd = opened_fd.as_raw_fd();
fd = opened_file.as_raw_fd();
let mut buf: libc::stat = unsafe { std::mem::zeroed() };
if unsafe { libc::fstat(fd, &mut buf) } == -1 {
let esc = escape(args[optind]);

6
src/env_universal_common.rs
View File

@ -387,10 +387,10 @@ impl EnvUniversal {
return true;
}
let Ok(fd) = open_cloexec(&self.narrow_vars_path, OFlag::O_RDONLY, Mode::empty()) else {
let Ok(mut file) = open_cloexec(&self.narrow_vars_path, OFlag::O_RDONLY, Mode::empty())
else {
return false;
};
let mut file = File::from(fd);
FLOG!(uvar_file, "universal log reading from file");
self.load_from_fd(&mut file, callbacks);
@ -443,7 +443,7 @@ impl EnvUniversal {
loop {
let mut file =
match wopen_cloexec(&self.vars_path, flags, Mode::from_bits_truncate(0o644)) {
Ok(fd) => File::from(fd),
Ok(file) => file,
Err(nix::Error::EINTR) => continue,
Err(err) => {
if !O_EXLOCK.is_empty() {

5
src/exec.rs
View File

@ -367,9 +367,8 @@ pub fn is_thompson_shell_script(path: &CStr) -> bool {
}
let e = errno();
let mut res = false;
let fd = open_cloexec(path, OFlag::O_RDONLY | OFlag::O_NOCTTY, stat::Mode::empty());
if let Ok(fd) = fd {
let mut file = std::fs::File::from(fd);
if let Ok(mut file) = open_cloexec(path, OFlag::O_RDONLY | OFlag::O_NOCTTY, stat::Mode::empty())
{
let mut buf = [b'\0'; 256];
if let Ok(got) = file.read(&mut buf) {
if is_thompson_shell_payload(&buf[..got]) {

11
src/fds.rs
View File

@ -10,6 +10,7 @@ use libc::{c_int, EINTR, FD_CLOEXEC, F_GETFD, F_GETFL, F_SETFD, F_SETFL, O_NONBL
use nix::fcntl::FcntlArg;
use nix::{fcntl::OFlag, unistd};
use std::ffi::CStr;
use std::fs::File;
use std::io::{self, Read, Write};
use std::os::unix::prelude::*;
@ -228,12 +229,12 @@ pub fn wopen_cloexec(
pathname: &wstr,
flags: OFlag,
mode: nix::sys::stat::Mode,
) -> nix::Result<OwnedFd> {
) -> nix::Result<File> {
open_cloexec(wcs2zstring(pathname).as_c_str(), flags, mode)
}
/// Narrow versions of wopen_cloexec().
pub fn open_cloexec(path: &CStr, flags: OFlag, mode: nix::sys::stat::Mode) -> nix::Result<OwnedFd> {
pub fn open_cloexec(path: &CStr, flags: OFlag, mode: nix::sys::stat::Mode) -> nix::Result<File> {
// Port note: the C++ version of this function had a fallback for platforms where
// O_CLOEXEC is not supported, using fcntl. In 2023, this is no longer needed.
let saved_errno = errno();
@ -243,11 +244,11 @@ pub fn open_cloexec(path: &CStr, flags: OFlag, mode: nix::sys::stat::Mode) -> ni
// If it is that's our cancel signal, so we abort.
loop {
let ret = nix::fcntl::open(path, flags | OFlag::O_CLOEXEC, mode);
let ret = ret.map(|raw_fd| unsafe { OwnedFd::from_raw_fd(raw_fd) });
let ret = ret.map(|raw_fd| unsafe { File::from_raw_fd(raw_fd) });
match ret {
Ok(fd) => {
Ok(file) => {
set_errno(saved_errno);
return Ok(fd);
return Ok(file);
}
Err(err) => {
if err != nix::Error::EINTR || signal_check_cancel() != 0 {

52
src/history.rs
View File

@ -476,12 +476,8 @@ impl HistoryImpl {
let _profiler = TimeProfiler::new("load_old");
if let Some(filename) = history_filename(&self.name, L!("")) {
let mut file = {
let Ok(fd) = wopen_cloexec(&filename, OFlag::O_RDONLY, Mode::empty()) else {
return;
};
// TODO: Return `File` directly from wopen_cloexec
File::from(fd)
let Ok(mut file) = wopen_cloexec(&filename, OFlag::O_RDONLY, Mode::empty()) else {
return;
};
// Take a read lock to guard against someone else appending. This is released after
@ -492,7 +488,7 @@ impl HistoryImpl {
// 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.
let locked = unsafe { Self::maybe_lock_file(&file, LOCK_SH) };
let locked = unsafe { Self::maybe_lock_file(&mut file, LOCK_SH) };
self.file_contents = HistoryFileContents::create(&mut file);
self.history_file_id = if self.file_contents.is_some() {
file_id_for_fd(file.as_fd())
@ -556,18 +552,16 @@ impl HistoryImpl {
/// Given the fd of an existing history file, write a new history file to `dst_fd`.
/// Returns false on error, true on success
fn rewrite_to_temporary_file(&self, existing_fd: Option<&mut File>, dst_fd: RawFd) -> bool {
fn rewrite_to_temporary_file(&self, existing_file: Option<&mut File>, dst: &mut File) -> bool {
// We are reading FROM existing_fd and writing TO dst_fd
// dst_fd must be valid
assert!(dst_fd >= 0);
// Make an LRU cache to save only the last N elements.
let mut lru = LruCache::new(HISTORY_SAVE_MAX);
// Read in existing items (which may have changed out from underneath us, so don't trust our
// old file contents).
if let Some(existing_fd) = existing_fd {
if let Some(local_file) = HistoryFileContents::create(existing_fd) {
if let Some(existing_file) = existing_file {
if let Some(local_file) = HistoryFileContents::create(existing_file) {
let mut cursor = 0;
while let Some(offset) = local_file.offset_of_next_item(&mut cursor, None) {
// Try decoding an old item.
@ -618,7 +612,7 @@ impl HistoryImpl {
let mut buffer = Vec::with_capacity(HISTORY_OUTPUT_BUFFER_SIZE + 128);
for item in items {
append_history_item_to_buffer(&item, &mut buffer);
if let Err(e) = flush_to_fd(&mut buffer, dst_fd, HISTORY_OUTPUT_BUFFER_SIZE) {
if let Err(e) = flush_to_fd(&mut buffer, dst.as_raw_fd(), HISTORY_OUTPUT_BUFFER_SIZE) {
err = Some(e);
break;
}
@ -632,7 +626,7 @@ impl HistoryImpl {
false
} else {
flush_to_fd(&mut buffer, dst_fd, 0).is_ok()
flush_to_fd(&mut buffer, dst.as_raw_fd(), 0).is_ok()
}
}
@ -672,8 +666,7 @@ impl HistoryImpl {
&target_name,
OFlag::O_RDONLY | OFlag::O_CREAT,
HISTORY_FILE_MODE,
)
.map(File::from);
);
let orig_file_id = target_file_before
.as_ref()
@ -681,9 +674,7 @@ impl HistoryImpl {
.unwrap_or(INVALID_FILE_ID);
// Open any target file, but do not lock it right away
if !self
.rewrite_to_temporary_file(target_file_before.ok().as_mut(), tmp_file.as_raw_fd())
{
if !self.rewrite_to_temporary_file(target_file_before.ok().as_mut(), &mut tmp_file) {
// Failed to write, no good
break;
}
@ -693,8 +684,8 @@ impl HistoryImpl {
// If the open fails, then proceed; this may be because there is no current history
let mut new_file_id = INVALID_FILE_ID;
let target_fd_after = wopen_cloexec(&target_name, OFlag::O_RDONLY, Mode::empty());
if let Ok(target_fd_after) = target_fd_after.as_ref() {
let mut target_fd_after = wopen_cloexec(&target_name, OFlag::O_RDONLY, Mode::empty());
if let Ok(target_fd_after) = target_fd_after.as_mut() {
// critical to take the lock before checking file IDs,
// and hold it until after we are done replacing.
// Also critical to check the file at the path, NOT based on our fd.
@ -806,7 +797,7 @@ impl HistoryImpl {
// Limit our max tries so we don't do this forever.
let mut history_file = None;
for _i in 0..MAX_SAVE_TRIES {
let Ok(fd) = wopen_cloexec(
let Ok(mut file) = wopen_cloexec(
&history_path,
OFlag::O_WRONLY | OFlag::O_APPEND,
Mode::empty(),
@ -814,7 +805,6 @@ impl HistoryImpl {
// can't open, we're hosed
break;
};
let mut file = File::from(fd);
// Exclusive lock on the entire file. This is released when we close the file (below). This
// may fail on (e.g.) lockless NFS. If so, proceed as if it did not fail; the risk is that
@ -1100,17 +1090,15 @@ impl HistoryImpl {
old_file.push_utfstr(&self.name);
old_file.push_str("_history");
let Ok(src_fd) = wopen_cloexec(&old_file, OFlag::O_RDONLY, Mode::empty()) else {
let Ok(mut src_file) = wopen_cloexec(&old_file, OFlag::O_RDONLY, Mode::empty()) else {
return;
};
let mut src_fd = std::fs::File::from(src_fd);
// Clear must come after we've retrieved the new_file name, and before we open
// destination file descriptor, since it destroys the name and the file.
self.clear();
let Ok(dst_fd) = wopen_cloexec(
let Ok(mut dst_file) = wopen_cloexec(
&new_file,
OFlag::O_WRONLY | OFlag::O_CREAT,
HISTORY_FILE_MODE,
@ -1119,15 +1107,13 @@ impl HistoryImpl {
return;
};
let mut dst_fd = std::fs::File::from(dst_fd);
let mut buf = [0; libc::BUFSIZ as usize];
while let Ok(n) = src_fd.read(&mut buf) {
while let Ok(n) = src_file.read(&mut buf) {
if n == 0 {
break;
}
if dst_fd.write(&buf[..n]).is_err() {
if dst_file.write(&buf[..n]).is_err() {
// This message does not have high enough priority to be shown by default.
FLOG!(history_file, "Error when writing history file");
break;
@ -1317,10 +1303,10 @@ impl HistoryImpl {
/// # Safety
///
/// `fd` and `lock_type` must be valid arguments to `flock(2)`.
unsafe fn maybe_lock_file(fd: impl AsFd, lock_type: libc::c_int) -> bool {
unsafe fn maybe_lock_file(file: &mut File, lock_type: libc::c_int) -> bool {
assert!(lock_type & LOCK_UN == 0, "Do not use lock_file to unlock");
let raw_fd = fd.as_fd().as_raw_fd();
let raw_fd = file.as_raw_fd();
// Don't lock if it took too long before, if we are simulating a failing lock, or if our history
// is on a remote filesystem.

3
src/io.rs
View File

@ -666,8 +666,7 @@ impl IoChain {
let oflags = spec.oflags();
match wopen_cloexec(&path, oflags, OPEN_MASK) {
Ok(fd) => {
let file = File::from(fd);
Ok(file) => {
self.push(Arc::new(IoFile::new(spec.fd, file)));
}
Err(err) => {

4
src/reader.rs
View File

@ -4665,11 +4665,9 @@ impl ReaderData {
var.map_or_else(|| L!("~/.bash_history").to_owned(), |var| var.as_string());
expand_tilde(&mut path, self.vars());
let Ok(fd) = wopen_cloexec(&path, OFlag::O_RDONLY, Mode::empty()) else {
let Ok(file) = wopen_cloexec(&path, OFlag::O_RDONLY, Mode::empty()) else {
return;
};
let file = std::fs::File::from(fd);
self.history.populate_from_bash(BufReader::new(file));
}
}

14
src/tests/history.rs
View File

@ -594,14 +594,12 @@ fn test_history_formats() {
"echo foo".into(),
];
let test_history_imported_from_bash = History::with_name(L!("bash_import"));
let file = std::fs::File::from(
wopen_cloexec(
L!("tests/history_sample_bash"),
OFlag::O_RDONLY,
Mode::empty(),
)
.unwrap(),
);
let file = wopen_cloexec(
L!("tests/history_sample_bash"),
OFlag::O_RDONLY,
Mode::empty(),
)
.unwrap();
test_history_imported_from_bash.populate_from_bash(BufReader::new(file));
assert_eq!(test_history_imported_from_bash.get_history(), expected);
test_history_imported_from_bash.clear();