Skip to content

Commit

Permalink
Don't implement Clone on Dir, SignalFd, and PtyMaster
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
asomers committed Feb 2, 2021
1 parent a279f78 commit bae8711
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 @@ -23,6 +23,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Removed

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

- 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 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 e == Err(Error::Sys(Errno::EBADF)) {
panic!("Closing an invalid file descriptor!");
};
}
}

Expand Down

0 comments on commit bae8711

Please sign in to comment.