diff --git a/staging/vhost-device-sound/Cargo.toml b/staging/vhost-device-sound/Cargo.toml index 747d3b4b..09213392 100644 --- a/staging/vhost-device-sound/Cargo.toml +++ b/staging/vhost-device-sound/Cargo.toml @@ -6,6 +6,7 @@ description = "A virtio-sound device using the vhost-user protocol." repository = "https://github.com/rust-vmm/vhost-device" readme = "README.md" keywords = ["vhost", "sound", "virtio-sound", "virtio-snd", "virtio"] +categories = ["multimedia::audio"] license = "Apache-2.0 OR BSD-3-Clause" edition = "2018" diff --git a/staging/vhost-device-sound/src/audio_backends/alsa.rs b/staging/vhost-device-sound/src/audio_backends/alsa.rs index 929941dd..d219fda1 100644 --- a/staging/vhost-device-sound/src/audio_backends/alsa.rs +++ b/staging/vhost-device-sound/src/audio_backends/alsa.rs @@ -164,7 +164,7 @@ fn update_pcm( let period_frames = period_bytes / frame_size; - hwp.set_period_size(period_frames as i64, alsa::ValueOr::Less)?; + hwp.set_period_size(i64::from(period_frames), alsa::ValueOr::Less)?; // Online ALSA driver recommendations seem to be that the buffer should be at // least 2 * period_size. @@ -177,7 +177,7 @@ fn update_pcm( // > as well as the parameters set in the snd_pcm_hardware structure (in the driver). // // So, if the operation fails let's assume the ALSA runtime has set a better value. - if let Err(err) = hwp.set_buffer_size_near(2 * period_frames as i64) { + if let Err(err) = hwp.set_buffer_size_near(2 * i64::from(period_frames)) { log::error!("could not set buffer size {}: {}", 2 * period_frames, err); } @@ -287,7 +287,7 @@ fn write_samples_io( p: &alsa::PCM, streams: &Arc>>, stream_id: usize, - io: &mut alsa::pcm::IO, + io: &alsa::pcm::IO, ) -> AResult { let avail = match p.avail_update() { Ok(n) => n, @@ -331,7 +331,7 @@ fn write_samples_io( if buffer.pos as u32 >= buffer.desc_len() { stream.buffers.pop_front(); } - p.bytes_to_frames(read_bytes as isize) + p.bytes_to_frames(isize::try_from(read_bytes).unwrap()) .try_into() .unwrap_or_default() })?; @@ -352,7 +352,7 @@ fn read_samples_io( p: &alsa::PCM, streams: &Arc>>, stream_id: usize, - io: &mut alsa::pcm::IO, + io: &alsa::pcm::IO, ) -> AResult { let avail = match p.avail_update() { Ok(n) => n, @@ -394,7 +394,8 @@ fn read_samples_io( .map(std::num::NonZeroUsize::get) { frames_read += frames; - let n_bytes = usize::try_from(p.frames_to_bytes(frames as i64)).unwrap_or_default(); + let n_bytes = + usize::try_from(p.frames_to_bytes(frames.try_into().unwrap())).unwrap_or_default(); if buffer .write_input(&intermediate_buf[0..n_bytes]) .expect("Could not write data to guest memory") @@ -404,7 +405,7 @@ fn read_samples_io( } } - let bytes_read = p.frames_to_bytes(frames_read as i64); + let bytes_read = p.frames_to_bytes(frames_read.try_into().unwrap()); if buffer.pos as u32 >= buffer.desc_len() || bytes_read == 0 { stream.buffers.pop_front(); } @@ -456,9 +457,9 @@ fn alsa_worker( continue 'empty_buffers; } } else { - let mut io = lck.io_bytes(); + let io = lck.io_bytes(); // Direct mode unavailable, use alsa-lib's mmap emulation instead - if write_samples_io(&lck, &streams, stream_id, &mut io)? { + if write_samples_io(&lck, &streams, stream_id, &io)? { continue 'empty_buffers; } } @@ -475,8 +476,8 @@ fn alsa_worker( continue 'empty_buffers; } } else { - let mut io = lck.io_bytes(); - if read_samples_io(&lck, &streams, stream_id, &mut io)? { + let io = lck.io_bytes(); + if read_samples_io(&lck, &streams, stream_id, &io)? { continue 'empty_buffers; } } @@ -505,6 +506,7 @@ impl AlsaBackend { Self { sender, streams } } + #[allow(clippy::cognitive_complexity)] fn run( streams: Arc>>, receiver: Receiver, diff --git a/staging/vhost-device-sound/src/audio_backends/null.rs b/staging/vhost-device-sound/src/audio_backends/null.rs index df9f01d3..ef5aa227 100644 --- a/staging/vhost-device-sound/src/audio_backends/null.rs +++ b/staging/vhost-device-sound/src/audio_backends/null.rs @@ -37,7 +37,7 @@ mod tests { let streams = Arc::new(RwLock::new(vec![Stream::default()])); let null_backend = NullBackend::new(streams.clone()); - assert!(null_backend.write(0).is_ok()); + null_backend.write(0).unwrap(); let streams = streams.read().unwrap(); assert_eq!(streams[0].buffers.len(), 0); @@ -48,7 +48,7 @@ mod tests { let streams = Arc::new(RwLock::new(vec![Stream::default()])); let null_backend = NullBackend::new(streams.clone()); - assert!(null_backend.read(0).is_ok()); + null_backend.read(0).unwrap(); // buffer lengths should remain unchanged let streams = streams.read().unwrap(); diff --git a/staging/vhost-device-sound/src/audio_backends/pipewire.rs b/staging/vhost-device-sound/src/audio_backends/pipewire.rs index a2fe72ef..e22b6b29 100644 --- a/staging/vhost-device-sound/src/audio_backends/pipewire.rs +++ b/staging/vhost-device-sound/src/audio_backends/pipewire.rs @@ -3,7 +3,7 @@ use std::{ collections::HashMap, - convert::TryInto, + convert::TryFrom, mem::size_of, ptr, sync::{Arc, RwLock}, @@ -70,6 +70,8 @@ unsafe impl Send for PwBackend {} // is protected with a lock. unsafe impl Sync for PwBackend {} +// FIXME: make PwBackend impl Send on all fields. +#[allow(clippy::non_send_fields_in_send_ty)] pub struct PwBackend { pub stream_params: Arc>>, thread_loop: ThreadLoop, @@ -274,7 +276,7 @@ impl AudioBackend for PwBackend { _ => 44100, }, flags: 0, - channels: params.channels as u32, + channels: u32::from(params.channels), position: pos, }; @@ -362,8 +364,10 @@ impl AudioBackend for PwBackend { }; let mut buf_pos = buffer.pos; - let avail = (buffer.desc_len() as usize - buf_pos) as i32; - let n_bytes = n_samples.min(avail.try_into().unwrap()); + let avail = usize::try_from(buffer.desc_len()) + .unwrap() + .saturating_sub(buf_pos); + let n_bytes = n_samples.min(avail); let p = &slice[start..start + n_bytes]; if buffer @@ -400,13 +404,15 @@ impl AudioBackend for PwBackend { let mut start = buffer.pos; - let avail = (buffer.desc_len() - start as u32) as i32; + let avail = usize::try_from(buffer.desc_len()) + .unwrap() + .saturating_sub(start); - if avail < n_bytes as i32 { - n_bytes = avail.try_into().unwrap(); + if avail < n_bytes { + n_bytes = avail; } let p = &mut slice[0..n_bytes]; - if avail <= 0 { + if avail == 0 { // SAFETY: We have assured above that the pointer is not // null // safe to zero-initialize the pointer. @@ -435,8 +441,8 @@ impl AudioBackend for PwBackend { }; let chunk = data.chunk_mut(); *chunk.offset_mut() = 0; - *chunk.stride_mut() = frame_size as _; - *chunk.size_mut() = n_bytes as _; + *chunk.stride_mut() = i32::try_from(frame_size).unwrap(); + *chunk.size_mut() = u32::try_from(n_bytes).unwrap(); } }; } @@ -582,7 +588,7 @@ mod tests { mem.write_obj::(hdr, desc_out.addr()).unwrap(); vq.desc_table().store(index, desc_out).unwrap(); - next_addr += desc_out.len() as u64; + next_addr += u64::from(desc_out.len()); index += 1; // In response descriptor @@ -635,19 +641,19 @@ mod tests { let pw_backend = PwBackend::new(stream_params); assert_eq!(pw_backend.stream_hash.read().unwrap().len(), 0); assert_eq!(pw_backend.stream_listener.read().unwrap().len(), 0); - assert!(pw_backend.prepare(0).is_ok()); - assert!(pw_backend.start(0).is_ok()); - assert!(pw_backend.stop(0).is_ok()); + pw_backend.prepare(0).unwrap(); + pw_backend.start(0).unwrap(); + pw_backend.stop(0).unwrap(); let msg = ctrlmsg(); - assert!(pw_backend.set_parameters(0, msg).is_ok()); + pw_backend.set_parameters(0, msg).unwrap(); let release_msg = ctrlmsg(); - assert!(pw_backend.release(0, release_msg).is_ok()); - assert!(pw_backend.write(0).is_ok()); + pw_backend.release(0, release_msg).unwrap(); + pw_backend.write(0).unwrap(); let streams = streams.read().unwrap(); assert_eq!(streams[0].buffers.len(), 0); - assert!(pw_backend.read(0).is_ok()); + pw_backend.read(0).unwrap(); } #[test] diff --git a/staging/vhost-device-sound/src/device.rs b/staging/vhost-device-sound/src/device.rs index dd89fdd1..97e7b944 100644 --- a/staging/vhost-device-sound/src/device.rs +++ b/staging/vhost-device-sound/src/device.rs @@ -124,6 +124,7 @@ impl VhostUserSoundThread { Ok(false) } + #[allow(clippy::cognitive_complexity)] fn process_control( &self, vring: &VringRwLock, @@ -209,6 +210,7 @@ impl VhostUserSoundThread { for i in chmaps.iter().skip(start_id).take(count) { buf.extend_from_slice(i.as_slice()); } + drop(chmaps); desc_chain .memory() .write_slice(&buf, desc_response.addr()) @@ -244,6 +246,7 @@ impl VhostUserSoundThread { for i in jacks.iter().skip(start_id).take(count) { buf.extend_from_slice(i.as_slice()); } + drop(jacks); desc_chain .memory() .write_slice(&buf, desc_response.addr()) @@ -297,6 +300,7 @@ impl VhostUserSoundThread { p.channels_max = s.channels_max; buf.extend_from_slice(p.as_slice()); } + drop(streams); desc_chain .memory() .write_slice(&buf, desc_response.addr()) @@ -519,10 +523,9 @@ impl VhostUserSoundThread { IoState::WaitingBufferForStreamId(stream_id) if descriptor.len() as usize == size_of::() => { - let mut streams = self.streams.write().unwrap(); - for b in std::mem::take(&mut buffers) { - streams[stream_id as usize].buffers.push_back(b); - } + self.streams.write().unwrap()[stream_id as usize] + .buffers + .extend(std::mem::take(&mut buffers).into_iter()); state = IoState::Done; } IoState::Ready @@ -574,7 +577,7 @@ impl VhostUserSoundThread { } if !stream_ids.is_empty() { - let b = audio_backend.write().unwrap(); + let b = audio_backend.read().unwrap(); match direction { Direction::Output => { for id in stream_ids { @@ -653,8 +656,8 @@ impl VhostUserSoundBackend { streams_no, )?), RwLock::new(VhostUserSoundThread::new( - chmaps.clone(), - jacks.clone(), + chmaps, + jacks, vec![RX_QUEUE_IDX], streams.clone(), streams_no, @@ -662,8 +665,8 @@ impl VhostUserSoundBackend { ] } else { vec![RwLock::new(VhostUserSoundThread::new( - chmaps.clone(), - jacks.clone(), + chmaps, + jacks, vec![ CONTROL_QUEUE_IDX, EVENT_QUEUE_IDX, @@ -858,7 +861,6 @@ mod tests { let thread = VhostUserSoundThread::new(chmaps, jacks, queue_indexes, streams.clone(), streams_no); - assert!(thread.is_ok()); let mut t = thread.unwrap(); // Mock memory @@ -874,21 +876,18 @@ mod tests { ]; let audio_backend = - RwLock::new(alloc_audio_backend(config.audio_backend, streams.clone()).unwrap()); - assert!(t - .handle_event(CONTROL_QUEUE_IDX, &vrings, &audio_backend) - .is_ok()); + RwLock::new(alloc_audio_backend(config.audio_backend, streams).unwrap()); + t.handle_event(CONTROL_QUEUE_IDX, &vrings, &audio_backend) + .unwrap(); let vring = VringRwLock::new(mem, 0x1000).unwrap(); vring.set_queue_info(0x100, 0x200, 0x300).unwrap(); vring.set_queue_ready(true); - assert!(t.process_control(&vring, &audio_backend).is_ok()); - assert!(t - .process_io(&vring, &audio_backend, Direction::Output) - .is_ok()); - assert!(t - .process_io(&vring, &audio_backend, Direction::Input) - .is_ok()); + t.process_control(&vring, &audio_backend).unwrap(); + t.process_io(&vring, &audio_backend, Direction::Output) + .unwrap(); + t.process_io(&vring, &audio_backend, Direction::Input) + .unwrap(); } #[test] @@ -909,15 +908,14 @@ mod tests { ); let audio_backend = - RwLock::new(alloc_audio_backend(config.audio_backend, streams.clone()).unwrap()); + RwLock::new(alloc_audio_backend(config.audio_backend, streams).unwrap()); let vring = VringRwLock::new(mem, 0x1000).unwrap(); vring.set_queue_info(0x100, 0x200, 0x300).unwrap(); vring.set_queue_ready(true); - assert!(t.process_control(&vring, &audio_backend).is_err()); - assert!(t - .process_io(&vring, &audio_backend, Direction::Output) - .is_err()); + t.process_control(&vring, &audio_backend).unwrap_err(); + t.process_io(&vring, &audio_backend, Direction::Output) + .unwrap_err(); } #[test] @@ -962,7 +960,7 @@ mod tests { .unwrap(); vrings[RX_QUEUE_IDX as usize].set_queue_ready(true); - assert!(backend.update_memory(mem).is_ok()); + backend.update_memory(mem).unwrap(); let queues_per_thread = backend.queues_per_thread(); assert_eq!(queues_per_thread.len(), 1); @@ -976,19 +974,15 @@ mod tests { exit.unwrap().write(1).unwrap(); let ret = backend.handle_event(CONTROL_QUEUE_IDX, EventSet::IN, &vrings, 0); - assert!(ret.is_ok()); assert!(!ret.unwrap()); let ret = backend.handle_event(EVENT_QUEUE_IDX, EventSet::IN, &vrings, 0); - assert!(ret.is_ok()); assert!(!ret.unwrap()); let ret = backend.handle_event(TX_QUEUE_IDX, EventSet::IN, &vrings, 0); - assert!(ret.is_ok()); assert!(!ret.unwrap()); let ret = backend.handle_event(RX_QUEUE_IDX, EventSet::IN, &vrings, 0); - assert!(ret.is_ok()); assert!(!ret.unwrap()); test_dir.close().unwrap(); diff --git a/staging/vhost-device-sound/src/lib.rs b/staging/vhost-device-sound/src/lib.rs index c6410ce9..e9fa0a97 100644 --- a/staging/vhost-device-sound/src/lib.rs +++ b/staging/vhost-device-sound/src/lib.rs @@ -1,5 +1,34 @@ // Manos Pitsidianakis // SPDX-License-Identifier: Apache-2.0 or BSD-3-Clause +// +#![deny( + /* groups */ + clippy::correctness, + clippy::suspicious, + clippy::complexity, + clippy::perf, + clippy::style, + clippy::nursery, + //* restriction */ + clippy::dbg_macro, + clippy::rc_buffer, + clippy::as_underscore, + clippy::assertions_on_result_states, + //* pedantic */ + clippy::cast_lossless, + clippy::cast_possible_wrap, + clippy::ptr_as_ptr, + clippy::bool_to_int_with_if, + clippy::borrow_as_ptr, + clippy::case_sensitive_file_extension_comparisons, + clippy::cast_lossless, + clippy::cast_ptr_alignment, + clippy::naive_bytecount +)] +#![allow( + clippy::significant_drop_in_scrutinee, + clippy::significant_drop_tightening +)] pub mod audio_backends; pub mod device; @@ -133,7 +162,7 @@ pub enum BackendType { Alsa, } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct InvalidControlMessage(u32); impl std::fmt::Display for InvalidControlMessage { @@ -253,7 +282,7 @@ pub struct SoundConfig { impl SoundConfig { /// Create a new instance of the SoundConfig struct, containing the /// parameters to be fed into the sound-backend server. - pub fn new(socket: String, multi_thread: bool, audio_backend: BackendType) -> Self { + pub const fn new(socket: String, multi_thread: bool, audio_backend: BackendType) -> Self { Self { socket, multi_thread, @@ -267,7 +296,7 @@ impl SoundConfig { String::from(&self.socket) } - pub fn get_audio_backend(&self) -> BackendType { + pub const fn get_audio_backend(&self) -> BackendType { self.audio_backend } } @@ -364,7 +393,7 @@ mod tests { let config = SoundConfig::new(SOCKET_PATH.to_string(), false, BackendType::Null); - let backend = Arc::new(VhostUserSoundBackend::new(config.clone()).unwrap()); + let backend = Arc::new(VhostUserSoundBackend::new(config).unwrap()); let daemon = VhostUserDaemon::new( String::from("vhost-device-sound"), backend.clone(), diff --git a/staging/vhost-device-sound/src/stream.rs b/staging/vhost-device-sound/src/stream.rs index fe6db4ce..89e12555 100644 --- a/staging/vhost-device-sound/src/stream.rs +++ b/staging/vhost-device-sound/src/stream.rs @@ -9,7 +9,7 @@ use vm_memory::{Address, Bytes, Le32, Le64}; use crate::{virtio_sound::*, Direction, IOMessage, SUPPORTED_FORMATS, SUPPORTED_RATES}; /// Stream errors. -#[derive(Debug, ThisError, PartialEq)] +#[derive(Debug, ThisError, PartialEq, Eq)] pub enum Error { #[error("Guest driver request an invalid stream state transition from {0} to {1}.")] InvalidStateTransition(PCMState, PCMState), @@ -93,7 +93,7 @@ type Result = std::result::Result; /// | | | | | /// | |<----------| | | /// ``` -#[derive(Debug, Default, Copy, Clone, PartialEq)] +#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] pub enum PCMState { #[default] #[doc(alias = "VIRTIO_SND_R_PCM_SET_PARAMS")] @@ -365,7 +365,7 @@ mod tests { mem.write_obj::(hdr, desc_out.addr()).unwrap(); vq.desc_table().store(index, desc_out).unwrap(); - next_addr += desc_out.len() as u64; + next_addr += u64::from(desc_out.len()); index += 1; // In response descriptor @@ -413,11 +413,9 @@ mod tests { let mut state = PCMState::new(); assert_eq!(state, PCMState::SetParameters); - assert!(state.set_parameters().is_ok()); state.set_parameters().unwrap(); assert_eq!(state, PCMState::SetParameters); - assert!(state.prepare().is_ok()); state.prepare().unwrap(); assert_eq!(state, PCMState::Prepare); @@ -432,7 +430,6 @@ mod tests { // Attempt to transition from set_params state to Release state let result = state.release(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -442,7 +439,6 @@ mod tests { ); let result = state.start(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -452,7 +448,6 @@ mod tests { ); let result = state.stop(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -463,7 +458,6 @@ mod tests { state.prepare().unwrap(); let result = state.stop(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -474,7 +468,6 @@ mod tests { state.start().unwrap(); let result = state.set_parameters(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -484,7 +477,6 @@ mod tests { ); let result = state.release(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -494,7 +486,6 @@ mod tests { ); let result = state.prepare(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -505,7 +496,6 @@ mod tests { state.stop().unwrap(); let result = state.set_parameters(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -515,7 +505,6 @@ mod tests { ); let result = state.prepare(); - assert!(result.is_err()); assert_eq!( result, Err(Error::InvalidStateTransition( @@ -555,7 +544,6 @@ mod tests { ); let mut buf = vec![0; 5]; - let result = buffer.read_output(&mut buf); - assert!(result.is_ok()); + buffer.read_output(&mut buf).unwrap(); } }