usb: remove unused/abused fd argument from EventHandler::on_event

None of instances of EventHandler::on_event actually used the fd. The
PollfdChangeHandler::remove_poll_fd callback fabricated a potentially
valid fd (0), which went undetected because nobody used it.
Additionally, using RawFds almost always requires unsafe and should be
avoided.

CQ-DEPEND=CL:1522214
BUG=chromium:831850
TEST=cargo test

Change-Id: I095edbcad317e4832b1fb29fd08d602fbde4fd5d
Reviewed-on: https://chromium-review.googlesource.com/1525135
Commit-Ready: Jingkui Wang <jkwang@google.com>
Tested-by: Jingkui Wang <jkwang@google.com>
Reviewed-by: Jingkui Wang <jkwang@google.com>

6 files changed, 20 insertions, 17 deletions

diff --git a/devices/src/usb/host_backend/context.rs b/devices/src/usb/host_backend/context.rs
index 260aad1..d4c60e3 100644
--- a/devices/src/usb/host_backend/context.rs
+++ b/devices/src/usb/host_backend/context.rs
@@ -116,7 +116,7 @@ struct LibUsbEventHandler {
 }
 
 impl EventHandler for LibUsbEventHandler {
-    fn on_event(&self, _fd: RawFd) -> std::result::Result<(), ()> {
+    fn on_event(&self) -> std::result::Result<(), ()> {
         self.context.handle_events_nonblock();
         Ok(())
     }
@@ -141,7 +141,7 @@ impl LibUsbPollfdChangeHandler for PollfdChangeHandler {
 
     fn remove_poll_fd(&self, fd: RawFd) {
         if let Some(h) = self.event_handler.upgrade() {
-            match h.on_event(0) {
+            match h.on_event() {
                 Ok(()) => {}
                 Err(e) => error!("cannot handle event: {:?}", e),
             }
diff --git a/devices/src/usb/host_backend/host_backend_device_provider.rs b/devices/src/usb/host_backend/host_backend_device_provider.rs
index 95b52fd..32c5e9a 100644
--- a/devices/src/usb/host_backend/host_backend_device_provider.rs
+++ b/devices/src/usb/host_backend/host_backend_device_provider.rs
@@ -143,7 +143,7 @@ impl ProviderInner {
         }
     }
 
-    fn on_event_helper(&self, _fd: RawFd) -> Result<()> {
+    fn on_event_helper(&self) -> Result<()> {
         let cmd = self.sock.recv().map_err(Error::ReadControlSock)?;
         match cmd {
             UsbControlCommand::AttachDevice {
@@ -315,8 +315,8 @@ impl ProviderInner {
 }
 
 impl EventHandler for ProviderInner {
-    fn on_event(&self, fd: RawFd) -> std::result::Result<(), ()> {
-        self.on_event_helper(fd).map_err(|e| {
+    fn on_event(&self) -> std::result::Result<(), ()> {
+        self.on_event_helper().map_err(|e| {
             error!("host backend device provider failed: {}", e);
             ()
         })
diff --git a/devices/src/usb/xhci/intr_resample_handler.rs b/devices/src/usb/xhci/intr_resample_handler.rs
index 202ed1a..8c7ff99 100644
--- a/devices/src/usb/xhci/intr_resample_handler.rs
+++ b/devices/src/usb/xhci/intr_resample_handler.rs
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 use super::interrupter::Interrupter;
-use std::os::unix::io::RawFd;
 use std::sync::Arc;
 use sync::Mutex;
 use sys_util::{EventFd, WatchingEvents};
@@ -39,7 +38,7 @@ impl IntrResampleHandler {
     }
 }
 impl EventHandler for IntrResampleHandler {
-    fn on_event(&self, _fd: RawFd) -> Result<(), ()> {
+    fn on_event(&self) -> Result<(), ()> {
         match self.resample_evt.read() {
             Ok(_) => {}
             Err(e) => {
diff --git a/devices/src/usb/xhci/ring_buffer_controller.rs b/devices/src/usb/xhci/ring_buffer_controller.rs
index 7aed102..6452a57 100644
--- a/devices/src/usb/xhci/ring_buffer_controller.rs
+++ b/devices/src/usb/xhci/ring_buffer_controller.rs
@@ -5,7 +5,6 @@
 use super::ring_buffer_stop_cb::RingBufferStopCallback;
 use super::xhci_abi::*;
 use std::fmt::{self, Display};
-use std::os::unix::io::RawFd;
 use std::sync::{Arc, MutexGuard};
 use sync::Mutex;
 use utils::{self, EventHandler, EventLoop};
@@ -184,7 +183,7 @@ impl<T> EventHandler for RingBufferController<T>
 where
     T: 'static + TransferDescriptorHandler + Send,
 {
-    fn on_event(&self, _fd: RawFd) -> std::result::Result<(), ()> {
+    fn on_event(&self) -> std::result::Result<(), ()> {
         // `self.event` triggers ring buffer controller to run, the value read is not important.
         match self.event.read() {
             Ok(_) => {}
diff --git a/devices/src/utils/async_job_queue.rs b/devices/src/utils/async_job_queue.rs
index 223b96d..8f9f4a3 100644
--- a/devices/src/utils/async_job_queue.rs
+++ b/devices/src/utils/async_job_queue.rs
@@ -5,7 +5,6 @@
 use super::{Error, Result};
 use super::{EventHandler, EventLoop};
 use std::mem;
-use std::os::unix::io::RawFd;
 use std::sync::Arc;
 use sync::Mutex;
 use sys_util::{EventFd, WatchingEvents};
@@ -41,7 +40,7 @@ impl AsyncJobQueue {
 }
 
 impl EventHandler for AsyncJobQueue {
-    fn on_event(&self, _fd: RawFd) -> std::result::Result<(), ()> {
+    fn on_event(&self) -> std::result::Result<(), ()> {
         // We want to read out the event, but the value is not important.
         match self.evt.read() {
             Ok(_) => {}
diff --git a/devices/src/utils/event_loop.rs b/devices/src/utils/event_loop.rs
index f27c19c..99fe1c9 100644
--- a/devices/src/utils/event_loop.rs
+++ b/devices/src/utils/event_loop.rs
@@ -65,7 +65,7 @@ pub struct EventLoop {
 
 /// Interface for event handler.
 pub trait EventHandler: Send + Sync {
-    fn on_event(&self, fd: RawFd) -> std::result::Result<(), ()>;
+    fn on_event(&self) -> std::result::Result<(), ()>;
 }
 
 impl EventLoop {
@@ -123,7 +123,7 @@ impl EventLoop {
                             Some(handler) => {
                                 // Drop lock before triggering the event.
                                 drop(locked);
-                                match handler.on_event(fd) {
+                                match handler.on_event() {
                                     Ok(()) => {}
                                     Err(_) => {
                                         error!("event loop stopping due to handle event error");
@@ -199,18 +199,18 @@ impl EventLoop {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
     use std::sync::{Arc, Condvar, Mutex};
     use sys_util::EventFd;
 
     struct EventLoopTestHandler {
         val: Mutex<u8>,
         cvar: Condvar,
+        evt: EventFd,
     }
 
     impl EventHandler for EventLoopTestHandler {
-        fn on_event(&self, fd: RawFd) -> std::result::Result<(), ()> {
-            let _ = unsafe { EventFd::from_raw_fd(fd).read() };
+        fn on_event(&self) -> std::result::Result<(), ()> {
+            self.evt.read().unwrap();
             *self.val.lock().unwrap() += 1;
             self.cvar.notify_one();
             Ok(())
@@ -230,9 +230,15 @@ mod tests {
         let h = Arc::new(EventLoopTestHandler {
             val: Mutex::new(0),
             cvar: Condvar::new(),
+            evt,
         });
         let t: Arc<EventHandler> = h.clone();
-        l.add_event(&evt, WatchingEvents::empty().set_read(), Arc::downgrade(&t));
+        l.add_event(
+            &h.evt,
+            WatchingEvents::empty().set_read(),
+            Arc::downgrade(&t),
+        )
+        .unwrap();
         self_evt.write(1).unwrap();
         let _ = h.cvar.wait(h.val.lock().unwrap()).unwrap();
         l.stop();