From 8a1f47d6c4a42eba2221db858206afbb84401b97 Mon Sep 17 00:00:00 2001 From: Bart Massey Date: Tue, 12 May 2020 18:18:32 -0700 Subject: [PATCH 1/3] brought up to date with current Rust ecosystem * Bumped bitflags version to > 1 to avoid try!() warnings. Edited bitflags code for compatibility. * Replaced mem_transmute() with Box::from_raw() in stream.rs as suggested by comments there. * Cleaned up all compiler warnings. * Cleaned up all Clippy warnings. --- Cargo.toml | 4 ++-- src/lib.rs | 8 +++---- src/stream.rs | 64 ++++++++++++++++++++++++--------------------------- 3 files changed, 36 insertions(+), 40 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd8da53..f1fea0b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "portaudio-rs" -version = "0.3.1" +version = "0.3.2" authors = ["mvdnes "] description = "PortAudio bindings for Rust" license = "MIT" @@ -9,6 +9,6 @@ license = "MIT" name = "portaudio_rs" [dependencies] -bitflags = "0.3" +bitflags = "1" libc = "0.2" portaudio-sys = { path = "portaudio-sys", version = "0.1" } diff --git a/src/lib.rs b/src/lib.rs index 9eaf16a..366e8ad 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,15 +9,15 @@ //! ``` //! fn demo() -> portaudio_rs::PaResult //! { -//! let stream = try!(portaudio_rs::stream::Stream::open_default( +//! let stream = portaudio_rs::stream::Stream::open_default( //! 0, // input channels //! 1, // output channels //! 44100.0, // sample rate //! portaudio_rs::stream::FRAMES_PER_BUFFER_UNSPECIFIED, //! None // no callback -//! )); +//! )?; //! -//! try!(stream.start()); +//! stream.start()?; //! //! let mut phase = 0.0f32; //! let mut buffer = Vec::with_capacity(44100); @@ -30,7 +30,7 @@ //! if phase > 1.0 { phase -= 2.0; } //! } //! -//! try!(stream.write(&buffer)); +//! stream.write(&buffer)?; //! //! Ok(()) //! } diff --git a/src/stream.rs b/src/stream.rs index 4a51628..ddc710c 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -29,10 +29,10 @@ pub enum StreamCallbackResult } /// Callback to consume, process or generate audio -pub type StreamCallback<'a, I, O> = FnMut(&[I], &mut [O], StreamTimeInfo, StreamCallbackFlags) -> StreamCallbackResult + 'a; +pub type StreamCallback<'a, I, O> = dyn FnMut(&[I], &mut [O], StreamTimeInfo, StreamCallbackFlags) -> StreamCallbackResult + 'a; /// Callback to be fired when a StreamCallback is stopped -pub type StreamFinishedCallback<'a> = FnMut() + 'a; +pub type StreamFinishedCallback<'a> = dyn FnMut() + 'a; struct StreamUserData<'a, I, O> { @@ -71,41 +71,41 @@ impl StreamTimeInfo bitflags!( #[doc="Flags indicating the status of the callback"] - flags StreamCallbackFlags: u64 { + pub struct StreamCallbackFlags: u64 { #[doc="Indicates that the callback has inserted one or more zeroes since not enough data was available"] - const INPUT_UNDERFLOW = 0x01, + const INPUT_UNDERFLOW = 0x01; #[doc="Indicates that the callback has discarded some data"] - const INPUT_OVERFLOW = 0x02, + const INPUT_OVERFLOW = 0x02; #[doc="Indicates that extra data was inserted in the output since there was not engough available"] - const OUTPUT_UNDERFLOW = 0x04, + const OUTPUT_UNDERFLOW = 0x04; #[doc="Indicates that certain data was discarded since there was no room"] - const OUTPUT_OVERFLOW = 0x08, + const OUTPUT_OVERFLOW = 0x08; #[doc="Some or all of the output data will be used to prime the stream, input data may be zero"] - const PRIMING_OUTPUT = 0x10 + const PRIMING_OUTPUT = 0x10; } ); bitflags!( #[doc="Flags used to control the behavior of a stream"] - flags StreamFlags: u64 { + pub struct StreamFlags: u64 { #[doc="Disable clipping of out of range samples"] - const CLIP_OFF = 0x00000001, + const CLIP_OFF = 0x0000_0001; #[doc="Disable dithering"] - const DITHER_OFF = 0x00000002, + const DITHER_OFF = 0x0000_0002; #[doc="Request that a full duplex stream will not discard overflowed input samples. The frames_per_buffer must be set to unspecified (0)"] - const NEVER_DROP_INPUT = 0x00000004, + const NEVER_DROP_INPUT = 0x0000_0004; #[doc="Call the stream callback to fill initial output buffers, rather than priming the buffers with silence"] - const PRIME_OUTPUT_BUFFERS_USING_STREAM_CALLBACK = 0x00000008, + const PRIME_OUTPUT_BUFFERS_USING_STREAM_CALLBACK = 0x0000_0008; #[doc="Range for platform specific flags. Not all of the upper 16 bits need to be set at the same time."] - const PLATFORM_SPECIFIC = 0xFFFF0000 + const PLATFORM_SPECIFIC = 0xFFFF_0000; } ); @@ -116,8 +116,7 @@ extern "C" fn stream_callback(input: *const c_void, status_flags: ll::PaStreamCallbackFlags, user_data: *mut c_void) -> ::libc::c_int { - // TODO: use Box::from_raw once it is stable - let mut stream_data: Box> = unsafe { mem::transmute(user_data) }; + let mut stream_data: Box> = unsafe { Box::from_raw(user_data as *mut StreamUserData) }; let input_buffer: &[I] = unsafe { @@ -147,12 +146,10 @@ extern "C" fn stream_callback(input: *const c_void, extern "C" fn stream_finished_callback(user_data: *mut c_void) { - // TODO: use Box::from_raw once it is stable - let mut stream_data: Box> = unsafe { mem::transmute(user_data) }; - match stream_data.finished_callback + let mut stream_data: Box> = unsafe { Box::from_raw(user_data as *mut StreamUserData) }; + if let Some(ref mut f) = stream_data.finished_callback { - Some(ref mut f) => (*f)(), - None => {}, + (*f)(); }; mem::forget(stream_data); @@ -167,11 +164,11 @@ pub trait SampleType /// Should return the PortAudio flag which corresponds to the type fn sample_format() -> u64; } -impl SampleType for f32 { fn sample_format() -> u64 { 0x00000001 } } -impl SampleType for i32 { fn sample_format() -> u64 { 0x00000002 } } -impl SampleType for i16 { fn sample_format() -> u64 { 0x00000008 } } -impl SampleType for i8 { fn sample_format() -> u64 { 0x00000010 } } -impl SampleType for u8 { fn sample_format() -> u64 { 0x00000020 } } +impl SampleType for f32 { fn sample_format() -> u64 { 0x0000_0001 } } +impl SampleType for i32 { fn sample_format() -> u64 { 0x0000_0002 } } +impl SampleType for i16 { fn sample_format() -> u64 { 0x0000_0008 } } +impl SampleType for i8 { fn sample_format() -> u64 { 0x0000_0010 } } +impl SampleType for u8 { fn sample_format() -> u64 { 0x0000_0020 } } #[cfg(test)] fn get_sample_size() -> Result @@ -226,7 +223,7 @@ impl<'a, T: SampleType> Stream<'a, T, T> { num_input: num_input_channels, num_output: num_output_channels, - callback: callback, + callback, finished_callback: None, }); let mut pa_stream = ::std::ptr::null_mut(); @@ -247,7 +244,7 @@ impl<'a, T: SampleType> Stream<'a, T, T> match to_pa_result(code) { - Ok(()) => Ok(Stream { pa_stream: pa_stream, + Ok(()) => Ok(Stream { pa_stream, user_data: userdata, inputs: num_input_channels, outputs: num_output_channels, @@ -299,7 +296,7 @@ impl<'a, I: SampleType, O: SampleType> Stream<'a, I, O> { num_input: input_cnt, num_output: output_cnt, - callback: callback, + callback, finished_callback: None, }); @@ -320,8 +317,8 @@ impl<'a, I: SampleType, O: SampleType> Stream<'a, I, O> match to_pa_result(result) { - Ok(()) => Ok(Stream { pa_stream: pa_stream, - user_data: user_data, + Ok(()) => Ok(Stream { pa_stream, + user_data, inputs: input_cnt, outputs: output_cnt, }), @@ -493,10 +490,9 @@ impl<'a, I: SampleType, O: SampleType> Drop for Stream<'a, I, O> fn drop(&mut self) { debug_assert!(self.user_data.num_output == self.outputs); //userdata should not be garbled - match self.close() + if let Err(v) = self.close() { - Err(v) => { let _ = write!(&mut ::std::io::stderr(), "Stream drop error: {:?}\n", v); }, - Ok(_) => {}, + let _ = writeln!(&mut ::std::io::stderr(), "Stream drop error: {:?}", v); }; } } From 47f14009f7100d207a52d5a8e6cf2f49a1e8c78d Mon Sep 17 00:00:00 2001 From: Bart Massey Date: Tue, 12 May 2020 18:46:16 -0700 Subject: [PATCH 2/3] fixed drop on callback panic; closes Issue #20 (RUSTSEC/CVE) Two uses of mem::forget() after user callbacks were replaced with Box::leak() before the user callbacks to ensure that unowned memory was not improperly freed on panic. --- src/stream.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/stream.rs b/src/stream.rs index ddc710c..7c073f3 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -4,7 +4,6 @@ use ll; use pa::{PaError, PaResult}; use device::DeviceIndex; use util::{to_pa_result, pa_time_to_duration, duration_to_pa_time}; -use std::mem; use std::time::Duration; use libc::{c_void, c_ulong}; use std::io::prelude::*; @@ -116,7 +115,8 @@ extern "C" fn stream_callback(input: *const c_void, status_flags: ll::PaStreamCallbackFlags, user_data: *mut c_void) -> ::libc::c_int { - let mut stream_data: Box> = unsafe { Box::from_raw(user_data as *mut StreamUserData) }; + // We do not want to deallocate this memory since it is owned by other user code. So leak the box. + let stream_data: &mut StreamUserData = Box::leak( unsafe { Box::from_raw(user_data as *mut StreamUserData) } ); let input_buffer: &[I] = unsafe { @@ -139,20 +139,17 @@ extern "C" fn stream_callback(input: *const c_void, None => StreamCallbackResult::Abort, }; - mem::forget(stream_data); - result as i32 } extern "C" fn stream_finished_callback(user_data: *mut c_void) { - let mut stream_data: Box> = unsafe { Box::from_raw(user_data as *mut StreamUserData) }; + // We do not want to deallocate this memory since it is owned by other user code. So leak the box. + let stream_data: &mut StreamUserData = Box::leak( unsafe { Box::from_raw(user_data as *mut StreamUserData) } ); if let Some(ref mut f) = stream_data.finished_callback { (*f)(); }; - - mem::forget(stream_data); } /// Types that are allowed to be used as samples in a Stream From f282e8c9c37b86859f5a3c3f032cedc6b6a650da Mon Sep 17 00:00:00 2001 From: Bart Massey Date: Tue, 12 May 2020 19:43:18 -0700 Subject: [PATCH 3/3] cleaned up compiler and Clippy warnings in examples --- examples/demo.rs | 6 +++--- examples/test.rs | 54 +++++++++++++++++++++--------------------------- 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/examples/demo.rs b/examples/demo.rs index 2eac7f7..e4bac3f 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -24,11 +24,11 @@ fn print_devs() fn demo() -> portaudio::PaResult { - let stream = try!(portaudio::stream::Stream::open_default(1, 1, 44100.0, portaudio::stream::FRAMES_PER_BUFFER_UNSPECIFIED, None)); + let stream = portaudio::stream::Stream::open_default(1, 1, 44100.0, portaudio::stream::FRAMES_PER_BUFFER_UNSPECIFIED, None)?; - try!(stream.start()); + stream.start()?; - let input = try!(stream.read(44100)); + let input = stream.read(44100)?; let mut phase = 0.0f32; let mut buffer = Vec::with_capacity(44100 * SECONDS); diff --git a/examples/test.rs b/examples/test.rs index b04a943..81316b2 100644 --- a/examples/test.rs +++ b/examples/test.rs @@ -15,36 +15,30 @@ fn main() fn print_info() { - match hostapi::get_count() + if let Ok(api_count) = hostapi::get_count() { - Ok(api_count) => { - for i in 0 .. api_count + for i in 0 .. api_count + { + let name = match hostapi::get_info(i) { - let name = match hostapi::get_info(i) - { - None => "???".to_string(), - Some(ha) => ha.name, - }; - println!("api {}: {}", i, name); - } - }, - _ => {}, + None => "???".to_string(), + Some(ha) => ha.name, + }; + println!("api {}: {}", i, name); + } } - match device::get_count() + if let Ok(device_count) = device::get_count() { - Ok(device_count) => { - for i in 0 .. device_count + for i in 0 .. device_count + { + let name = match device::get_info(i) { - let name = match device::get_info(i) - { - None => "???".to_string(), - Some(d) => d.name, - }; - println!("dev {}: {}", i, name); - } - }, - _ => {}, + None => "???".to_string(), + Some(d) => d.name, + }; + println!("dev {}: {}", i, name); + } } } @@ -59,11 +53,11 @@ fn callback_demo() { let callback = Box::new(|_input: &[f32], output: &mut [f32], _time: stream::StreamTimeInfo, _flags: stream::StreamCallbackFlags| -> stream::StreamCallbackResult { - static mut lp: f32 = 0.0; - static mut rp: f32 = 0.0; + static mut LP: f32 = 0.0; + static mut RP: f32 = 0.0; - let mut left_phase = unsafe { lp }; - let mut right_phase = unsafe { rp }; + let mut left_phase = unsafe { LP }; + let mut right_phase = unsafe { RP }; for i in 0 .. output.len() / 2 { @@ -77,8 +71,8 @@ fn callback_demo() if right_phase >= 1.0 { right_phase -= 2.0; } } - unsafe { lp = left_phase; } - unsafe { rp = right_phase; } + unsafe { LP = left_phase; } + unsafe { RP = right_phase; } stream::StreamCallbackResult::Continue });