Skip to content

Commit

Permalink
Expose MIDI device names as CString because they might not be UTF-8
Browse files Browse the repository at this point in the history
  • Loading branch information
helgoboss committed Jul 25, 2024
1 parent 7343040 commit 074aabc
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 20 deletions.
5 changes: 3 additions & 2 deletions main/high/src/midi_input_device.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::Reaper;
use reaper_medium::{MidiInput, MidiInputDeviceId, ReaperString};
use reaper_medium::{MidiInput, MidiInputDeviceId};
#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};
use std::ffi::CString;

#[derive(Clone, Copy, PartialEq, Eq, Debug)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize), serde(transparent))]
Expand All @@ -18,7 +19,7 @@ impl MidiInputDevice {
self.id
}

pub fn name(self) -> Option<ReaperString> {
pub fn name(self) -> Option<CString> {
Reaper::get()
.medium_reaper()
.get_midi_input_name(self.id, 64)
Expand Down
5 changes: 3 additions & 2 deletions main/high/src/midi_output_device.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::Reaper;
use reaper_medium::{MidiOutput, MidiOutputDeviceId, ReaperString};
use reaper_medium::{MidiOutput, MidiOutputDeviceId};
#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};
use std::ffi::CString;

#[derive(Clone, Copy, PartialEq, Eq, Debug)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize), serde(transparent))]
Expand All @@ -18,7 +19,7 @@ impl MidiOutputDevice {
self.id
}

pub fn name(self) -> Option<ReaperString> {
pub fn name(self) -> Option<CString> {
Reaper::get()
.medium_reaper()
.get_midi_output_name(self.id, 64)
Expand Down
27 changes: 17 additions & 10 deletions main/medium/src/reaper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ use reaper_low::raw::GUID;

use crate::ptr_wrappers::require_hwnd_panic;
use crate::util::{
create_passing_c_str, with_buffer, with_string_buffer, with_string_buffer_prefilled,
create_passing_c_str, with_buffer, with_string_buffer, with_string_buffer_cstring,
with_string_buffer_prefilled,
};
use camino::{Utf8Path, Utf8PathBuf};
use enumflags2::BitFlags;
Expand Down Expand Up @@ -3738,10 +3739,11 @@ impl<UsageScope> Reaper<UsageScope> {
name: None,
}
} else {
let (name, is_present) = with_string_buffer(buffer_size, |buffer, max_size| unsafe {
self.low
.GetMIDIInputName(device_id.to_raw(), buffer, max_size)
});
let (name, is_present) =
with_string_buffer_cstring(buffer_size, |buffer, max_size| unsafe {
self.low
.GetMIDIInputName(device_id.to_raw(), buffer, max_size)
});
if name.is_empty() {
return GetMidiDevNameResult {
is_present,
Expand Down Expand Up @@ -3778,10 +3780,11 @@ impl<UsageScope> Reaper<UsageScope> {
name: None,
}
} else {
let (name, is_present) = with_string_buffer(buffer_size, |buffer, max_size| unsafe {
self.low
.GetMIDIOutputName(device_id.to_raw(), buffer, max_size)
});
let (name, is_present) =
with_string_buffer_cstring(buffer_size, |buffer, max_size| unsafe {
self.low
.GetMIDIOutputName(device_id.to_raw(), buffer, max_size)
});
if name.is_empty() {
return GetMidiDevNameResult {
is_present,
Expand Down Expand Up @@ -8628,7 +8631,11 @@ pub struct GetMidiDevNameResult {
/// Whether the device is currently connected.
pub is_present: bool,
/// Name of the device (only if name requested and device known).
pub name: Option<ReaperString>,
///
/// This is a [`CString`] instead of a [`ReaperString`] because REAPER versions at least up to < v7.19 have
/// a bug that can cause the resulting string to not be proper UTF-8 if the MIDI device contains non-ASCII
/// characters. So one must be careful when interpreting the result.
pub name: Option<CString>,
}

#[derive(Clone, Eq, PartialEq, Hash, Debug)]
Expand Down
3 changes: 2 additions & 1 deletion main/medium/src/string_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ impl ReaperString {
ReaperString(inner)
}

// Don't make this public!
// Don't make this public! If we do, we need to mark it as unsafe (it's like saying "I vow that the given CString
// is valid UTF-8").
pub(crate) fn new(inner: CString) -> ReaperString {
ReaperString(inner)
}
Expand Down
19 changes: 14 additions & 5 deletions main/medium/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ pub fn with_string_buffer<T>(
max_size: u32,
fill_buffer: impl FnOnce(*mut c_char, i32) -> T,
) -> (ReaperString, T) {
let (cstring, result) = with_string_buffer_cstring(max_size, fill_buffer);
(ReaperString::new(cstring), result)
}

pub fn with_string_buffer_cstring<T>(
max_size: u32,
fill_buffer: impl FnOnce(*mut c_char, i32) -> T,
) -> (CString, T) {
// Using with_capacity() here wouldn't be correct because it leaves the vector length at zero.
let vec: Vec<u8> = vec![0; max_size as usize];
with_string_buffer_internal(vec, max_size, fill_buffer)
Expand All @@ -32,19 +40,20 @@ pub fn with_string_buffer_prefilled<'a, T>(
) -> (ReaperString, T) {
let mut vec = Vec::from(prefill.into().as_reaper_str().as_c_str().to_bytes());
vec.resize(max_size as usize, 0);
with_string_buffer_internal(vec, max_size, fill_buffer)
let (cstring, result) = with_string_buffer_internal(vec, max_size, fill_buffer);
(ReaperString::new(cstring), result)
}

pub fn with_string_buffer_internal<T>(
fn with_string_buffer_internal<T>(
vec: Vec<u8>,
max_size: u32,
fill_buffer: impl FnOnce(*mut c_char, i32) -> T,
) -> (ReaperString, T) {
) -> (CString, T) {
let c_string = unsafe { CString::from_vec_unchecked(vec) };
let raw = c_string.into_raw();
let result = fill_buffer(raw, max_size as i32);
let string = unsafe { ReaperString::new(CString::from_raw(raw)) };
(string, result)
let cstring = unsafe { CString::from_raw(raw) };
(cstring, result)
}

pub fn with_buffer<T>(
Expand Down

0 comments on commit 074aabc

Please sign in to comment.