Add proper error handling to the pulseaudio backend and ensure that no invalid pointers are passed to pulseaudio

Cargo.lock

@@ -1,10 +1,3 @@
-[root]
-name = "librespot-protocol"
-version = "0.1.0"
-dependencies = [
- "protobuf 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
 [[package]]
 name = "aho-corasick"
 version = "0.6.3"
@@ -271,6 +264,7 @@ dependencies = [
  "futures 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "hyper 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.29 (registry+https://github.com/rust-lang/crates.io-index)",
  "libpulse-sys 0.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "librespot-audio 0.1.0",
  "librespot-core 0.1.0",
@@ -353,6 +347,13 @@ dependencies = [
  "protobuf 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "librespot-protocol"
+version = "0.1.0"
+dependencies = [
+ "protobuf 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "linear-map"
 version = "1.2.0"

Cargo.toml

@@ -52,6 +52,7 @@ url = "1.3"
 alsa            = { git = "https://github.com/plietar/rust-alsa", optional = true }
 portaudio-rs    = { version = "0.3.0", optional = true }
 libpulse-sys    = { version = "0.0.0", optional = true }
+libc            = { version = "0.2", optional = true }
 
 [build-dependencies]
 rand            = "0.3.13"
@@ -61,7 +62,7 @@ protobuf_macros = { git = "https://github.com/plietar/rust-protobuf-macros", fea
 [features]
 alsa-backend = ["alsa"]
 portaudio-backend = ["portaudio-rs"]
-pulseaudio-backend = ["libpulse-sys"]
+pulseaudio-backend = ["libpulse-sys", "libc"]
 
 with-tremor = ["librespot-audio/with-tremor"]
 with-lewton = ["librespot-audio/with-lewton"]

src/audio_backend/pulseaudio.rs

@@ -2,8 +2,10 @@ use super::{Open, Sink};
 use std::io;
 use libpulse_sys::*;
 use std::ptr::{null, null_mut};
-use std::mem::{transmute};
 use std::ffi::CString;
+use std::ffi::CStr;
+use std::mem;
+use libc;
 
 pub struct PulseAudioSink {
     s    : *mut pa_simple,
@@ -12,6 +14,39 @@ pub struct PulseAudioSink {
     desc : CString
 }
 
+fn call_pulseaudio<T, F, FailCheck>(f: F, fail_check: FailCheck, kind: io::ErrorKind) -> io::Result<T> where
+    T: Copy,
+    F: Fn(*mut libc::c_int) -> T,
+    FailCheck: Fn(T) -> bool,
+{
+    let mut error: libc::c_int = 0;
+    let ret = f(&mut error);
+    if fail_check(ret) {
+        let err_cstr = unsafe { CStr::from_ptr(pa_strerror(error)) };
+        let errstr =  err_cstr.to_string_lossy().into_owned();
+        Err(io::Error::new(kind, errstr))
+    } else {
+        Ok(ret)
+    }
+}
+
+impl PulseAudioSink {
+    fn free_connection(&mut self) {
+        if self.s != null_mut() {
+            unsafe {
+                pa_simple_free(self.s);
+            }
+            self.s = null_mut();
+        }
+    }
+}
+
+impl Drop for PulseAudioSink {
+    fn drop(&mut self) {
+        self.free_connection();
+    }
+}
+
 impl Open for PulseAudioSink {
    fn open(device: Option<String>) -> PulseAudioSink {
         debug!("Using PulseAudio sink");
@@ -27,7 +62,7 @@ impl Open for PulseAudioSink {
         };
         
         let name = CString::new("librespot").unwrap();
-        let description = CString::new("A spoty client library").unwrap();
+        let description = CString::new("Spotify endpoint").unwrap();
 
         PulseAudioSink {
             s: null_mut(),
@@ -41,7 +76,8 @@ impl Open for PulseAudioSink {
 impl Sink for PulseAudioSink {
     fn start(&mut self) -> io::Result<()> {
         if self.s == null_mut() {
-            self.s = unsafe {
+            self.s = call_pulseaudio(
+                |err| unsafe {
                     pa_simple_new(null(),               // Use the default server.
                                   self.name.as_ptr(),   // Our application's name.
                                   PA_STREAM_PLAYBACK,
@@ -50,29 +86,33 @@ impl Sink for PulseAudioSink {
                                   &self.ss,             // Our sample format.
                                   null(),               // Use default channel map
                                   null(),               // Use default buffering attributes.
-                              null_mut(),           // Ignore error code.
-                )
-            };
-            assert!(self.s != null_mut());
+                                  err)
+                },
+                |ptr| ptr == null_mut(),
+                io::ErrorKind::ConnectionRefused)?;
         }
         Ok(())
     }
 
     fn stop(&mut self) -> io::Result<()> {
-        unsafe {
-            pa_simple_free(self.s);
-        }
-        self.s = null_mut();
+        self.free_connection();
         Ok(())
     }
 
     fn write(&mut self, data: &[i16]) -> io::Result<()> {
-        unsafe {
-            let ptr = transmute(data.as_ptr());
-            let bytes = data.len() as usize * 2;
-            pa_simple_write(self.s, ptr, bytes, null_mut());
-        };
-        
+        if self.s == null_mut() {
+            Err(io::Error::new(io::ErrorKind::NotConnected, "Not connected to pulseaudio"))
+        }
+        else {
+            let ptr = data.as_ptr() as *const libc::c_void;
+            let len = data.len() as usize * mem::size_of::<i16>();
+            call_pulseaudio(
+                |err| unsafe {
+                    pa_simple_write(self.s, ptr, len, err)
+                },
+                |ret| ret < 0,
+                io::ErrorKind::BrokenPipe)?;
             Ok(())
         }
+    }
 }

src/lib.rs

@@ -34,6 +34,9 @@ extern crate portaudio_rs;
 #[cfg(feature = "libpulse-sys")]
 extern crate libpulse_sys;
 
+#[cfg(feature = "libc")]
+extern crate libc;
+
 pub mod audio_backend;
 pub mod discovery;
 pub mod keymaster;