From d8aefd598cd69afff27627d253d0311c186449c3 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 29 Jul 2024 10:17:56 +0000 Subject: [PATCH 1/4] Make field private. It is not used outside the module and it should not be directly accessed anyway --- src/shims/unix/fd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index 0fffecd99d..d6399489c9 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -213,7 +213,7 @@ impl FileDescriptor { /// The file descriptor table #[derive(Debug)] pub struct FdTable { - pub fds: BTreeMap, + fds: BTreeMap, } impl VisitProvenance for FdTable { From 250b80996d6ec175b268927005070c7a7420a1bf Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 29 Jul 2024 11:46:54 +0000 Subject: [PATCH 2/4] Split out actual `FileDescriptor` creation --- src/shims/unix/fd.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index d6399489c9..85ff9f1fc6 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -192,6 +192,10 @@ impl FileDescription for NullOutput { pub struct FileDescriptor(Rc>>); impl FileDescriptor { + fn new(fd: impl FileDescription) -> Self { + FileDescriptor(Rc::new(RefCell::new(Box::new(fd)))) + } + pub fn borrow(&self) -> Ref<'_, dyn FileDescription> { Ref::map(self.0.borrow(), |fd| fd.as_ref()) } @@ -239,14 +243,14 @@ impl FdTable { fds } - /// Insert a file descriptor to the FdTable. - pub fn insert_fd(&mut self, fd: T) -> i32 { - let file_handle = FileDescriptor(Rc::new(RefCell::new(Box::new(fd)))); + /// Insert a new file description to the FdTable. + pub fn insert_fd(&mut self, fd: impl FileDescription) -> i32 { + let file_handle = FileDescriptor::new(fd); self.insert_fd_with_min_fd(file_handle, 0) } /// Insert a new FD that is at least `min_fd`. - pub fn insert_fd_with_min_fd(&mut self, file_handle: FileDescriptor, min_fd: i32) -> i32 { + fn insert_fd_with_min_fd(&mut self, file_handle: FileDescriptor, min_fd: i32) -> i32 { // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in // between used FDs, the find_map combinator will return it. If the first such unused FD // is after all other used FDs, the find_map combinator will return None, and we will use From eb6589dc767e776235800999d725a5b8aa246d57 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 29 Jul 2024 11:47:49 +0000 Subject: [PATCH 3/4] Rename `FileDescriptor` to `FileDescriptionRef` --- src/shims/unix/fd.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index 85ff9f1fc6..323b7371d4 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -189,11 +189,11 @@ impl FileDescription for NullOutput { } #[derive(Clone, Debug)] -pub struct FileDescriptor(Rc>>); +pub struct FileDescriptionRef(Rc>>); -impl FileDescriptor { +impl FileDescriptionRef { fn new(fd: impl FileDescription) -> Self { - FileDescriptor(Rc::new(RefCell::new(Box::new(fd)))) + FileDescriptionRef(Rc::new(RefCell::new(Box::new(fd)))) } pub fn borrow(&self) -> Ref<'_, dyn FileDescription> { @@ -217,7 +217,7 @@ impl FileDescriptor { /// The file descriptor table #[derive(Debug)] pub struct FdTable { - fds: BTreeMap, + fds: BTreeMap, } impl VisitProvenance for FdTable { @@ -245,12 +245,12 @@ impl FdTable { /// Insert a new file description to the FdTable. pub fn insert_fd(&mut self, fd: impl FileDescription) -> i32 { - let file_handle = FileDescriptor::new(fd); + let file_handle = FileDescriptionRef::new(fd); self.insert_fd_with_min_fd(file_handle, 0) } /// Insert a new FD that is at least `min_fd`. - fn insert_fd_with_min_fd(&mut self, file_handle: FileDescriptor, min_fd: i32) -> i32 { + fn insert_fd_with_min_fd(&mut self, file_handle: FileDescriptionRef, min_fd: i32) -> i32 { // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in // between used FDs, the find_map combinator will return it. If the first such unused FD // is after all other used FDs, the find_map combinator will return None, and we will use @@ -286,12 +286,12 @@ impl FdTable { Some(fd.borrow_mut()) } - pub fn dup(&self, fd: i32) -> Option { + pub fn dup(&self, fd: i32) -> Option { let fd = self.fds.get(&fd)?; Some(fd.clone()) } - pub fn remove(&mut self, fd: i32) -> Option { + pub fn remove(&mut self, fd: i32) -> Option { self.fds.remove(&fd) } From 1ecd1865adcf20af3ed299704a7bc4a3560eca2e Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 29 Jul 2024 11:50:01 +0000 Subject: [PATCH 4/4] Rename all `file_descriptor` variables to `file_description` to match the naming scheme of the types --- src/shims/unix/fd.rs | 8 ++++---- src/shims/unix/fs.rs | 40 ++++++++++++++++++++-------------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index 323b7371d4..74bb8eeb95 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -320,9 +320,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if new_fd != old_fd { // Close new_fd if it is previously opened. // If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive. - if let Some(file_descriptor) = this.machine.fds.fds.insert(new_fd, dup_fd) { + if let Some(file_description) = this.machine.fds.fds.insert(new_fd, dup_fd) { // Ignore close error (not interpreter's) according to dup2() doc. - file_descriptor.close(this.machine.communicate())?.ok(); + file_description.close(this.machine.communicate())?.ok(); } } Ok(new_fd) @@ -389,10 +389,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd = this.read_scalar(fd_op)?.to_i32()?; - let Some(file_descriptor) = this.machine.fds.remove(fd) else { + let Some(file_description) = this.machine.fds.remove(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - let result = file_descriptor.close(this.machine.communicate())?; + let result = file_description.close(this.machine.communicate())?; // return `0` if close is successful let result = result.map(|()| 0i32); Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index 6923b39733..6546cefd28 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -483,13 +483,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let communicate = this.machine.communicate(); - let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else { + let Some(mut file_description) = this.machine.fds.get_mut(fd) else { return Ok(Scalar::from_i64(this.fd_not_found()?)); }; - let result = file_descriptor + let result = file_description .seek(communicate, seek_from)? .map(|offset| i64::try_from(offset).unwrap()); - drop(file_descriptor); + drop(file_description); let result = this.try_unwrap_io_result(result)?; Ok(Scalar::from_i64(result)) @@ -1176,30 +1176,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(file_description) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; // FIXME: Support ftruncate64 for all FDs let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { + file_description.downcast_ref::().ok_or_else(|| { err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors") })?; if *writable { if let Ok(length) = length.try_into() { let result = file.set_len(length); - drop(file_descriptor); + drop(file_description); let result = this.try_unwrap_io_result(result.map(|_| 0i32))?; Ok(Scalar::from_i32(result)) } else { - drop(file_descriptor); + drop(file_description); let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; Ok(Scalar::from_i32(-1)) } } else { - drop(file_descriptor); + drop(file_description); // The file is not writable let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; @@ -1229,16 +1229,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn ffullsync_fd(&mut self, fd: i32) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(file_description) = this.machine.fds.get(fd) else { return Ok(this.fd_not_found()?); }; // Only regular files support synchronization. let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { + file_description.downcast_ref::().ok_or_else(|| { err_unsup_format!("`fsync` is only supported on file-backed file descriptors") })?; let io_result = maybe_sync_file(file, *writable, File::sync_all); - drop(file_descriptor); + drop(file_description); this.try_unwrap_io_result(io_result) } @@ -1254,16 +1254,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return this.fd_not_found(); } - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(file_description) = this.machine.fds.get(fd) else { return Ok(this.fd_not_found()?); }; // Only regular files support synchronization. let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { + file_description.downcast_ref::().ok_or_else(|| { err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors") })?; let io_result = maybe_sync_file(file, *writable, File::sync_data); - drop(file_descriptor); + drop(file_description); this.try_unwrap_io_result(io_result) } @@ -1302,18 +1302,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(file_description) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; // Only regular files support synchronization. let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { + file_description.downcast_ref::().ok_or_else(|| { err_unsup_format!( "`sync_data_range` is only supported on file-backed file descriptors" ) })?; let io_result = maybe_sync_file(file, *writable, File::sync_data); - drop(file_descriptor); + drop(file_description); Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) } @@ -1609,11 +1609,11 @@ impl FileMetadata { ecx: &mut MiriInterpCx<'tcx>, fd: i32, ) -> InterpResult<'tcx, Option> { - let Some(file_descriptor) = ecx.machine.fds.get(fd) else { + let Some(file_description) = ecx.machine.fds.get(fd) else { return ecx.fd_not_found().map(|_: i32| None); }; - let file = &file_descriptor + let file = &file_description .downcast_ref::() .ok_or_else(|| { err_unsup_format!( @@ -1623,7 +1623,7 @@ impl FileMetadata { .file; let metadata = file.metadata(); - drop(file_descriptor); + drop(file_description); FileMetadata::from_meta(ecx, metadata) }