sound: add clippy lint checks

Add basic lints to deny unidiomatic or potentially bad code.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
This commit is contained in:
Manos Pitsidianakis 2023-10-31 15:28:38 +02:00 committed by Viresh Kumar
parent 75ca1ee428
commit 90f547e98c
6 changed files with 90 additions and 75 deletions

View File

@ -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<RwLock<Vec<Stream>>>,
stream_id: usize,
io: &mut alsa::pcm::IO<u8>,
io: &alsa::pcm::IO<u8>,
) -> AResult<bool> {
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<RwLock<Vec<Stream>>>,
stream_id: usize,
io: &mut alsa::pcm::IO<u8>,
io: &alsa::pcm::IO<u8>,
) -> AResult<bool> {
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<RwLock<Vec<Stream>>>,
receiver: Receiver<AlsaAction>,

View File

@ -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();

View File

@ -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<RwLock<Vec<Stream>>>,
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,
};
@ -439,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();
}
};
}
@ -586,7 +588,7 @@ mod tests {
mem.write_obj::<R>(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
@ -639,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]

View File

@ -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::<VirtioSoundPcmStatus>() =>
{
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();

View File

@ -1,5 +1,34 @@
// Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
// 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(),

View File

@ -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<T> = std::result::Result<T, Error>;
/// | | | | |
/// | |<----------| | |
/// ```
#[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::<R>(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();
}
}