Skip to content

Commit

Permalink
Reduce use of unwrap/expect.
Browse files Browse the repository at this point in the history
Unwrap and expect have been reduced where possible to try to prevent the
possibilities of panicking.
  • Loading branch information
mdwn committed May 14, 2024
1 parent 8b93231 commit 6a0f174
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 74 deletions.
8 changes: 6 additions & 2 deletions src/audio/cpal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ impl Device {
// Stop only when we hit a frame boundary. This will prevent weird noises
// when stopping a song.
if cancel_handle.is_cancelled() && source.get_frame_position() == 0 {
tx.send(()).expect("error sending message");
if tx.send(()).is_err() {
error!("Error sending message")

Check warning on line 213 in src/audio/cpal.rs

View check run for this annotation

Codecov / codecov/patch

src/audio/cpal.rs#L213

Added line #L213 was not covered by tests
}
true
} else {
false
Expand Down Expand Up @@ -241,7 +243,9 @@ impl Device {
data_pos += 1;
}
None => {
tx.send(()).expect("error sending message");
if tx.send(()).is_err() {
error!("Error sending message")

Check warning on line 247 in src/audio/cpal.rs

View check run for this annotation

Codecov / codecov/patch

src/audio/cpal.rs#L247

Added line #L247 was not covered by tests
}
return;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ mod test {
/// Signals the next event to the monitor thread.
fn next_event(&self, event: TestEvent) {
{
let mut current_event = self.current_event.lock().unwrap();
let mut current_event = self.current_event.lock().expect("failed to get lock");
*current_event = event;
}
// Wait until the thread goes to receive the event.
Expand All @@ -187,7 +187,7 @@ mod test {
loop {
// Wait for next event to set the current event.
barrier.wait();
let current_event = current_event.lock().unwrap();
let current_event = current_event.lock().expect("failed to get lock");
// Let next event know that we got the event.
barrier.wait();
match *current_event {
Expand Down
84 changes: 47 additions & 37 deletions src/midi/midir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,24 @@ impl super::Device for Device {
let span = span!(Level::INFO, "wait for event (midir)");
let _enter = span.enter();

let mut event_connection = self.event_connection.lock().expect("Unable to get lock");
let mut event_connection = self.event_connection.lock().expect("unable to get lock");

Check warning on line 49 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L49

Added line #L49 was not covered by tests
if event_connection.is_some() {
return Err("Already watching events.".into());
}

info!("Watching MIDI events.");

if self.input_port.is_none() {
warn!("No MIDI output device configured, cannot listen for events.");
return Ok(());
}
let input_port = match self.input_port.as_ref() {
Some(input_port) => input_port,

Check warning on line 57 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L56-L57

Added lines #L56 - L57 were not covered by tests
None => {
warn!("No MIDI output device configured, cannot listen for events.");
return Ok(());

Check warning on line 60 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L59-L60

Added lines #L59 - L60 were not covered by tests
}
};

let input = MidiInput::new("mtrack player input")?;
*event_connection = Some(input.connect(
self.input_port.as_ref().unwrap(),
input_port,
"mtrack input watcher",
move |_, raw_event, _| {
if let Ok(event) = LiveEvent::parse(raw_event) {
Expand All @@ -85,7 +88,7 @@ impl super::Device for Device {
let event_connection = self
.event_connection
.lock()
.expect("Error getting mutex.")
.expect("error getting mutex")
.take();

mem::drop(event_connection);
Expand All @@ -101,20 +104,24 @@ impl super::Device for Device {
let span = span!(Level::INFO, "play song (midir)");
let _enter = span.enter();

if self.output_port.is_none() {
warn!(
song = song.name,
"No MIDI output device configured, cannot play song."
);
return Ok(());
}
let output_port = match self.output_port.as_ref() {
Some(output_port) => output_port,

Check warning on line 108 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L107-L108

Added lines #L107 - L108 were not covered by tests
None => {
warn!(

Check warning on line 110 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L110

Added line #L110 was not covered by tests
song = song.name,
"No MIDI output device configured, cannot play song."
);
return Ok(());

Check warning on line 114 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L114

Added line #L114 was not covered by tests
}
};

let maybe_sheet = song.midi_sheet()?;
if maybe_sheet.is_none() {
info!(song = song.name, "Song has no MIDI sheet.");
return Ok(());
}
let midi_sheet = maybe_sheet.unwrap();
let midi_sheet = match song.midi_sheet()? {
Some(midi_sheet) => midi_sheet,

Check warning on line 119 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L118-L119

Added lines #L118 - L119 were not covered by tests
None => {
info!(song = song.name, "Song has no MIDI sheet.");
return Ok(());

Check warning on line 122 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L121-L122

Added lines #L121 - L122 were not covered by tests
}
};
let output = MidiOutput::new("mtrack player output")?;

info!(
Expand All @@ -128,8 +135,7 @@ impl super::Device for Device {
let cancel_handle = cancel_handle.clone();

// Wrap the midir connection in a cancel connection so that we can stop playback.
let midir_connection =
output.connect(self.output_port.as_ref().unwrap(), "mtrack player")?;
let midir_connection = output.connect(output_port, "mtrack player")?;

Check warning on line 138 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L138

Added line #L138 was not covered by tests
let connection = CancelConnection {
connection: midir_connection,
cancel_handle: cancel_handle.clone(),
Expand Down Expand Up @@ -243,18 +249,21 @@ fn list_midir_devices() -> Result<Vec<Device>, Box<dyn Error>> {

for port in output_ports {
let name = output.port_name(&port)?;
if !devices.contains_key(&name) {
devices.insert(
name.clone(),
Device {
name: name.clone(),
input_port: None,
output_port: Some(port),
event_connection: Box::new(Mutex::new(None)),
},
);
} else {
devices.get_mut(&name).unwrap().output_port = Some(port);
match devices.get_mut(&name) {
Some(device) => {
device.output_port = Some(port);

Check warning on line 254 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L252-L254

Added lines #L252 - L254 were not covered by tests
}
None => {
devices.insert(
name.clone(),
Device {
name: name.clone(),
input_port: None,
output_port: Some(port),
event_connection: Box::new(Mutex::new(None)),

Check warning on line 263 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L257-L263

Added lines #L257 - L263 were not covered by tests
},
);
}
}
}

Expand Down Expand Up @@ -319,9 +328,10 @@ impl<T: Timer> Timer for AccurateTimer<T> {
self.last_instant = Some(last_instant.add(duration));

// Subtract the duration unless it would be an overflow. If so, use the original duration.
duration = duration
.checked_sub(Instant::now().duration_since(last_instant))
.unwrap_or(duration);
duration = match duration.checked_sub(Instant::now().duration_since(last_instant)) {
Some(duration) => duration,
None => duration,

Check warning on line 333 in src/midi/midir.rs

View check run for this annotation

Codecov / codecov/patch

src/midi/midir.rs#L331-L333

Added lines #L331 - L333 were not covered by tests
};
}
None => self.last_instant = Some(time::Instant::now()),
};
Expand Down
14 changes: 7 additions & 7 deletions src/midi/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl Device {
/// Sends the mock event through to the sender.
pub fn mock_event(&self, event: &Vec<u8>) {
{
let mut mutex_event = self.event.lock().expect("Unable to get event lock.");
let mut mutex_event = self.event.lock().expect("unable to get event lock");
*mutex_event = event.to_vec();
}
// Wait until the thread goes to receive the event.
Expand All @@ -68,7 +68,7 @@ impl Device {
let emit_called = self
.emit_called
.lock()
.expect("Unable to get emit called lock.");
.expect("unable to get emit called lock");
match emit_called.as_ref() {
Some(event) => Some(event.to_vec()),
None => None,
Expand All @@ -81,7 +81,7 @@ impl Device {
let mut emit_called = self
.emit_called
.lock()
.expect("Unable to get emit called lock.");
.expect("unable to get emit called lock");
*emit_called = None;
}
}
Expand All @@ -94,7 +94,7 @@ impl super::Device for Device {

/// Watches MIDI input for events and sends them to the given sender.
fn watch_events(&self, sender: Sender<Vec<u8>>) -> Result<(), Box<dyn Error>> {
let mut event_thread = self.event_thread.lock().expect("Unable to get lock");
let mut event_thread = self.event_thread.lock().expect("unable to get lock");
if event_thread.is_some() {
return Err("Already watching events.".into());
}
Expand All @@ -109,10 +109,10 @@ impl super::Device for Device {
if closed.load(Ordering::Relaxed) {
return;
}
let event = event.lock().expect("Unable to get event lock.");
let event = event.lock().expect("unable to get event lock");
sender
.blocking_send(event.to_vec())
.expect("Error sending event.");
.expect("error sending event");
}
barrier.wait();
}));
Expand Down Expand Up @@ -174,7 +174,7 @@ impl super::Device for Device {
let mut emit_called = self
.emit_called
.lock()
.expect("Unable to get emit called lock.");
.expect("unable to get emit called lock");

let mut buf: Vec<u8> = Vec::with_capacity(8);
midi_event.write(&mut buf)?;
Expand Down
35 changes: 19 additions & 16 deletions src/player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,17 +229,18 @@ impl Player {
let _enter = self.span.enter();

let mut join = self.join.lock().await;
if join.is_none() {
return Ok(false);
}
info!(
song = self.get_playlist().current().name,
"Waiting for song to finish.",
);

// Wait for the mutex to become available and immediately drop it.
(&mut join.as_mut().unwrap().join).await?;
Ok(true)
Ok(match join.as_mut() {
Some(join) => {
info!(
song = self.get_playlist().current().name,

Check warning on line 235 in src/player.rs

View check run for this annotation

Codecov / codecov/patch

src/player.rs#L232-L235

Added lines #L232 - L235 were not covered by tests
"Waiting for song to finish.",
);
// Wait for the mutex to become available and immediately drop it.
(&mut join.join).await?;
true

Check warning on line 240 in src/player.rs

View check run for this annotation

Codecov / codecov/patch

src/player.rs#L239-L240

Added lines #L239 - L240 were not covered by tests
}
None => false,

Check warning on line 242 in src/player.rs

View check run for this annotation

Codecov / codecov/patch

src/player.rs#L242

Added line #L242 was not covered by tests
})
}

/// Next goes to the next entry in the playlist.
Expand Down Expand Up @@ -276,10 +277,13 @@ impl Player {
pub async fn stop(&mut self) -> Result<(), Box<dyn Error>> {
let mut join = self.join.lock().await;

if join.is_none() {
info!("Player is not active, nothing to stop.");
return Ok(());
}
let join = match join.as_mut() {
Some(join) => join,
None => {
info!("Player is not active, nothing to stop.");
return Ok(());

Check warning on line 284 in src/player.rs

View check run for this annotation

Codecov / codecov/patch

src/player.rs#L283-L284

Added lines #L283 - L284 were not covered by tests
}
};

if self
.stop_run
Expand All @@ -296,7 +300,6 @@ impl Player {
);

// Cancel the playback.
let join = join.as_mut().unwrap();
join.cancel.cancel();
Ok(())
}
Expand Down
25 changes: 17 additions & 8 deletions src/playlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ impl fmt::Display for Playlist {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "Playlist ({} songs):", self.songs.len())?;
for song_name in self.songs.iter() {
let song: Arc<Song> = self.registry.get(song_name).unwrap();
writeln!(f, " - {} (Channels: {})", song.name, song.num_channels)?;
match self.registry.get(song_name) {
Ok(song) => writeln!(f, " - {} (Channels: {})", song.name, song.num_channels)?,
Err(_) => writeln!(f, " - {} (unable to find song)", song_name)?,
};
}

Ok(())
Expand All @@ -45,8 +47,7 @@ impl fmt::Display for Playlist {
impl Playlist {
/// Creates a new playlist.
pub fn new(song_names: Vec<String>, registry: Arc<Songs>) -> Result<Playlist, Box<dyn Error>> {
// Verify that each song in the playlist exists in the registry. This will allow us
// to unwrap subsequent song gets later without worrying about panicking.
// Verify that each song in the playlist exists in the registry.
for song_name in song_names.iter() {
registry.get(song_name)?;
}
Expand Down Expand Up @@ -79,12 +80,15 @@ impl Playlist {
pub fn next(&self) -> Arc<Song> {
let _enter = self.span.enter();

let mut position = self.position.write().unwrap();
let mut position = self.position.write().expect("unable to get lock");
if *position < self.songs.len() - 1 {
*position += 1;
}

let current = &self.registry.get(&self.songs[*position]).unwrap();
let current = &self
.registry
.get(&self.songs[*position])
.expect("unable to get song from the registry");

info!(
position = *position,
Expand Down Expand Up @@ -116,8 +120,13 @@ impl Playlist {

/// Return the song at the current position of the playlist.
pub fn current(&self) -> Arc<Song> {
let position = self.position.read().unwrap();
Arc::clone(&self.registry.get(&self.songs[*position]).unwrap())
let position = self.position.read().expect("unable to get lock");
Arc::clone(
&self
.registry
.get(&self.songs[*position])
.expect("unable to find song in the registry"),
)
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/songs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,10 @@ where
files_to_tracks.insert(file_path.clone(), Vec::new());
}

files_to_tracks.get_mut(file_path).unwrap().push(track);
files_to_tracks
.get_mut(file_path)
.expect("unable to get tracks")
.push(track);

Check warning on line 314 in src/songs.rs

View check run for this annotation

Codecov / codecov/patch

src/songs.rs#L312-L314

Added lines #L312 - L314 were not covered by tests
});

let num_channels = *track_mapping
Expand Down Expand Up @@ -502,7 +505,7 @@ where
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| {
Some((val + 1) % self.channels)
})
.expect("Got none from frame position update function");
.expect("got none from frame position update function");
}

sample
Expand Down

0 comments on commit 6a0f174

Please sign in to comment.