Repository: hannobraun/inotify-rs (language: rust)

Issue:
Implement `RawFd`-related traits
Implement the [`RawFd`-related traits](https://doc.rust-lang.org/std/os/unix/io/index.html) for `WatchDescriptor` and possibly `Inotify`.

`Inotify` currently has an `fd` method that allows access to the file descriptor. I've made that method `unsafe`, to signal that it could mess with the `Inotify` internals. I'm not sure now whether that's a good use of `unsafe`.

Maybe the method should just be replaced with `AsRawFd`/`IntoRawFd` implementations. `FromRawFd` should be implemented in any case.




Discussion / hints:
As has been pointed out in #59, the watch descriptor is not a file descriptor, which means no `RawFd`-related traits should be implemented for `WatchDescriptor`. The issue is still relevant however, as far as `Inotify` is concerned.

Fix patch:
diff --git a/inotify/src/lib.rs b/inotify/src/lib.rs
index 7897d41..2303904 100644
--- a/inotify/src/lib.rs
+++ b/inotify/src/lib.rs
@@ -32,7 +32,12 @@ use std::mem;
 use std::io;
 use std::io::ErrorKind;
 use std::os::unix::ffi::OsStrExt;
-use std::os::unix::io::RawFd;
+use std::os::unix::io::{
+    AsRawFd,
+    FromRawFd,
+    IntoRawFd,
+    RawFd,
+};
 use std::path::Path;
 use std::slice;
 use std::ffi::{
@@ -458,23 +463,6 @@ impl Inotify {
             _ => Err(io::Error::last_os_error()),
         }
     }
-
-
-    /// Access inotify file descriptor
-    ///
-    /// This method provides access to the inotify file descriptor. While this
-    /// is not required for any of the tasks that are covered by this API, it
-    /// might be necessary for providing additional features on top of it.
-    ///
-    /// # Safety
-    ///
-    /// This function is marked `unsafe`, as direct access to the file
-    /// descriptor allows for all kinds of actions that could cause `Inotify` to
-    /// no longer work correctly. Please be aware of what you're doing, and how
-    /// this might affect the inotify-rs code.
-    pub unsafe fn fd(&mut self) -> &mut RawFd {
-        &mut self.0
-    }
 }
 
 impl Drop for Inotify {
@@ -485,6 +473,26 @@ impl Drop for Inotify {
     }
 }
 
+impl AsRawFd for Inotify {
+    fn as_raw_fd(&self) -> RawFd {
+        self.0
+    }
+}
+
+impl FromRawFd for Inotify {
+    unsafe fn from_raw_fd(fd: RawFd) -> Self {
+        Inotify(fd)
+    }
+}
+
+impl IntoRawFd for Inotify {
+    fn into_raw_fd(self) -> RawFd {
+        let fd = self.0;
+        mem::forget(self);
+        fd
+    }
+}
+
 
 /// Contains the [`WatchMask`] flags
 ///


Test patch:
diff --git a/inotify/tests/main.rs b/inotify/tests/main.rs
index 955624d..9ce8f87 100644
--- a/inotify/tests/main.rs
+++ b/inotify/tests/main.rs
@@ -90,6 +90,29 @@ fn it_should_not_accept_watchdescriptors_from_other_instances() {
     assert_eq!(inotify.rm_watch(wd2).unwrap_err().kind(), ErrorKind::InvalidInput);
 }
 
+#[test]
+fn it_should_implement_raw_fd_traits_correctly() {
+    use std::os::unix::io::{
+        FromRawFd,
+        IntoRawFd,
+    };
+
+
+    let fd = Inotify::init()
+        .expect("Failed to initialize inotify instance")
+        .into_raw_fd();
+
+    // If `IntoRawFd` has been implemented naively, `Inotify`'s `Drop`
+    // implementation will have closed the inotify instance at this point. Let's
+    // make sure this didn't happen.
+    let mut inotify = unsafe { <Inotify as FromRawFd>::from_raw_fd(fd) };
+
+    let mut buffer = [0; 1024];
+    if let Err(error) = inotify.read_events(&mut buffer) {
+        panic!("Failed to add watch: {}", error);
+    }
+}
+
 
 struct TestDir {
     dir: TempDir,