From f5d8019c18ef565b237bbefea9c2dbdde23cd374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20B=C3=A4chler?= Date: Mon, 27 Nov 2017 20:01:30 +0100 Subject: [PATCH] Add proper error handling to the pulseaudio backend and ensure that no invalid pointers are passed to pulseaudio --- Cargo.lock | 15 +++--- Cargo.toml | 3 +- src/audio_backend/pulseaudio.rs | 92 +++++++++++++++++++++++---------- src/lib.rs | 3 ++ 4 files changed, 79 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8128db1..eaddea19 100644 --- a/Cargo.lock +++ b/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" diff --git a/Cargo.toml b/Cargo.toml index f4e63498..82bef079 100644 --- a/Cargo.toml +++ b/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"] diff --git a/src/audio_backend/pulseaudio.rs b/src/audio_backend/pulseaudio.rs index 5e3b8c18..e9f0039b 100644 --- a/src/audio_backend/pulseaudio.rs +++ b/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(f: F, fail_check: FailCheck, kind: io::ErrorKind) -> io::Result 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) -> 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,38 +76,43 @@ impl Open for PulseAudioSink { impl Sink for PulseAudioSink { fn start(&mut self) -> io::Result<()> { if self.s == null_mut() { - self.s = unsafe { - pa_simple_new(null(), // Use the default server. - self.name.as_ptr(), // Our application's name. - PA_STREAM_PLAYBACK, - null(), // Use the default device. - self.desc.as_ptr(), // desc of our stream. - &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()); + 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, + null(), // Use the default device. + self.desc.as_ptr(), // desc of our stream. + &self.ss, // Our sample format. + null(), // Use default channel map + null(), // Use default buffering attributes. + 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()); - }; - - Ok(()) + 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::(); + call_pulseaudio( + |err| unsafe { + pa_simple_write(self.s, ptr, len, err) + }, + |ret| ret < 0, + io::ErrorKind::BrokenPipe)?; + Ok(()) + } } } diff --git a/src/lib.rs b/src/lib.rs index b9c920ec..5eaab011 100644 --- a/src/lib.rs +++ b/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;