Skip to content

Commit 0bfa49a

Browse files
bors[bot]asomers
andauthored
Merge #1382
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>
2 parents 3b8180c + c9cb83a commit 0bfa49a

File tree

4 files changed

+14
-5
lines changed

4 files changed

+14
-5
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
2828

2929
### Removed
3030

31+
- `Dir`, `SignalFd`, and `PtyMaster` are no longer `Clone`.
32+
(#[1382](https://github.com/nix-rust/nix/pull/1382))
33+
3134
- Removed `SockLevel`, which hasn't been used for a few years
3235
(#[1362](https://github.com/nix-rust/nix/pull/1362))
3336

src/dir.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use libc::{dirent, readdir_r};
2525
/// * returns entries for `.` (current directory) and `..` (parent directory).
2626
/// * returns entries' names as a `CStr` (no allocation or conversion beyond whatever libc
2727
/// does).
28-
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
28+
#[derive(Debug, Eq, Hash, PartialEq)]
2929
pub struct Dir(
3030
ptr::NonNull<libc::DIR>
3131
);
@@ -85,7 +85,10 @@ impl AsRawFd for Dir {
8585

8686
impl Drop for Dir {
8787
fn drop(&mut self) {
88-
unsafe { libc::closedir(self.0.as_ptr()) };
88+
let e = Errno::result(unsafe { libc::closedir(self.0.as_ptr()) });
89+
if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
90+
panic!("Closing an invalid file descriptor!");
91+
};
8992
}
9093
}
9194

src/pty.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub struct ForkptyResult {
4343
/// While this datatype is a thin wrapper around `RawFd`, it enforces that the available PTY
4444
/// functions are given the correct file descriptor. Additionally this type implements `Drop`,
4545
/// so that when it's consumed or goes out of scope, it's automatically cleaned-up.
46-
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
46+
#[derive(Debug, Eq, Hash, PartialEq)]
4747
pub struct PtyMaster(RawFd);
4848

4949
impl AsRawFd for PtyMaster {

src/sys/signalfd.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub fn signalfd(fd: RawFd, mask: &SigSet, flags: SfdFlags) -> Result<RawFd> {
7979
/// Err(err) => (), // some error happend
8080
/// }
8181
/// ```
82-
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
82+
#[derive(Debug, Eq, Hash, PartialEq)]
8383
pub struct SignalFd(RawFd);
8484

8585
impl SignalFd {
@@ -116,7 +116,10 @@ impl SignalFd {
116116

117117
impl Drop for SignalFd {
118118
fn drop(&mut self) {
119-
let _ = unistd::close(self.0);
119+
let e = unistd::close(self.0);
120+
if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
121+
panic!("Closing an invalid file descriptor!");
122+
};
120123
}
121124
}
122125

0 commit comments

Comments
 (0)