Skip to content

Commit

Permalink
sound: add clippy lint checks
Browse files Browse the repository at this point in the history 
Add basic lints to deny unidiomatic or potentially bad code.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
  • Loading branch information
@epilys @vireshk
epilys authored and vireshk committed Nov 2, 2023
1 parent 75ca1ee commit 90f547e
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 75 deletions.
24 changes: 13 additions & 11 deletions staging/vhost-device-sound/src/audio_backends/alsa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
})?;
Expand All @@ -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,
Expand Down Expand Up @@ -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")
Expand All @@ -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();
}
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -505,6 +506,7 @@ impl AlsaBackend {
Self { sender, streams }
}

#[allow(clippy::cognitive_complexity)]
fn run(
streams: Arc<RwLock<Vec<Stream>>>,
receiver: Receiver<AlsaAction>,
Expand Down
4 changes: 2 additions & 2 deletions staging/vhost-device-sound/src/audio_backends/null.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand Down
24 changes: 13 additions & 11 deletions staging/vhost-device-sound/src/audio_backends/pipewire.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -274,7 +276,7 @@ impl AudioBackend for PwBackend {
_ => 44100,
},
flags: 0,
channels: params.channels as u32,
channels: u32::from(params.channels),
position: pos,
};

Expand Down Expand Up @@ -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();
}
};
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
56 changes: 25 additions & 31 deletions staging/vhost-device-sound/src/device.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ impl VhostUserSoundThread {
Ok(false)
}

#[allow(clippy::cognitive_complexity)]
fn process_control(
&self,
vring: &VringRwLock,
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -653,17 +656,17 @@ impl VhostUserSoundBackend {
streams_no,
)?),
RwLock::new(VhostUserSoundThread::new(
chmaps.clone(),
jacks.clone(),
chmaps,
jacks,
vec![RX_QUEUE_IDX],
streams.clone(),
streams_no,
)?),
]
} else {
vec![RwLock::new(VhostUserSoundThread::new(
chmaps.clone(),
jacks.clone(),
chmaps,
jacks,
vec![
CONTROL_QUEUE_IDX,
EVENT_QUEUE_IDX,
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down
Loading

0 comments on commit 90f547e

Please sign in to comment.