Skip to content

Commit

Permalink
Merge #1382
Browse files Browse the repository at this point in the history
1382: Don't implement Clone on Dir, SignalFd, and PtyMaster r=asomers a=asomers

Since they close their file descriptors on Drop, it's almost impossible
to use Clone without creating a double-close situation.

Also, check for EBADF in SignalFd::drop and Dir::drop.

Co-authored-by: Alan Somers <asomers@gmail.com>
  • Loading branch information
bors[bot] and asomers committed Feb 13, 2021
2 parents 3b8180c + c9cb83a commit 0bfa49a
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -28,6 +28,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Removed

- `Dir`, `SignalFd`, and `PtyMaster` are no longer `Clone`.
(#[1382](https://github.com/nix-rust/nix/pull/1382))

- Removed `SockLevel`, which hasn't been used for a few years
(#[1362](https://github.com/nix-rust/nix/pull/1362))

Expand Down
7 changes: 5 additions & 2 deletions src/dir.rs
Expand Up @@ -25,7 +25,7 @@ use libc::{dirent, readdir_r};
/// * returns entries for `.` (current directory) and `..` (parent directory).
/// * returns entries' names as a `CStr` (no allocation or conversion beyond whatever libc
/// does).
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[derive(Debug, Eq, Hash, PartialEq)]
pub struct Dir(
ptr::NonNull<libc::DIR>
);
Expand Down Expand Up @@ -85,7 +85,10 @@ impl AsRawFd for Dir {

impl Drop for Dir {
fn drop(&mut self) {
unsafe { libc::closedir(self.0.as_ptr()) };
let e = Errno::result(unsafe { libc::closedir(self.0.as_ptr()) });
if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
panic!("Closing an invalid file descriptor!");
};
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/pty.rs
Expand Up @@ -43,7 +43,7 @@ pub struct ForkptyResult {
/// While this datatype is a thin wrapper around `RawFd`, it enforces that the available PTY
/// functions are given the correct file descriptor. Additionally this type implements `Drop`,
/// so that when it's consumed or goes out of scope, it's automatically cleaned-up.
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[derive(Debug, Eq, Hash, PartialEq)]
pub struct PtyMaster(RawFd);

impl AsRawFd for PtyMaster {
Expand Down
7 changes: 5 additions & 2 deletions src/sys/signalfd.rs
Expand Up @@ -79,7 +79,7 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
/// Err(err) => (), // some error happend
/// }
/// ```
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[derive(Debug, Eq, Hash, PartialEq)]
pub struct SignalFd(RawFd);

impl SignalFd {
Expand Down Expand Up @@ -116,7 +116,10 @@ impl SignalFd {

impl Drop for SignalFd {
fn drop(&mut self) {
let _ = unistd::close(self.0);
let e = unistd::close(self.0);
if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
panic!("Closing an invalid file descriptor!");
};
}
}

Expand Down

0 comments on commit 0bfa49a

Please sign in to comment.