Skip to content

Commit

Permalink
unsafety in text fix
Browse files Browse the repository at this point in the history
  • Loading branch information
sanbox-irl committed Sep 15, 2023
1 parent ba02f22 commit cef617b
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 33 deletions.
11 changes: 9 additions & 2 deletions imgui/src/lib.rs
Expand Up @@ -721,9 +721,16 @@ impl Ui {
let handle = &mut *self.scratch_buffer().get();

handle.refresh_buffer();
let label_ptr = handle.push(label);
let label_start = handle.push(label);

let items_inner: Vec<_> = items.iter().map(|&v| handle.push(v)).collect();
// we do this in two allocations
let items_inner: Vec<usize> = items.iter().map(|&v| handle.push(v)).collect();
let items_inner: Vec<*const _> = items_inner
.into_iter()
.map(|v| handle.buffer.as_ptr().add(v) as *const _)
.collect();

let label_ptr = handle.buffer.as_ptr().add(label_start) as *const _;

(label_ptr, items_inner)
};
Expand Down
35 changes: 25 additions & 10 deletions imgui/src/string.rs
Expand Up @@ -24,7 +24,9 @@ impl UiBuffer {
/// Internal method to push a single text to our scratch buffer.
pub fn scratch_txt(&mut self, txt: impl AsRef<str>) -> *const sys::cty::c_char {
self.refresh_buffer();
self.push(txt)

let start_of_substr = self.push(txt);
unsafe { self.offset(start_of_substr) }
}

/// Internal method to push an option text to our scratch buffer.
Expand All @@ -42,7 +44,11 @@ impl UiBuffer {
txt_1: impl AsRef<str>,
) -> (*const sys::cty::c_char, *const sys::cty::c_char) {
self.refresh_buffer();
(self.push(txt_0), self.push(txt_1))

let first_offset = self.push(txt_0);
let second_offset = self.push(txt_1);

unsafe { (self.offset(first_offset), self.offset(second_offset)) }
}

/// Helper method, same as [`Self::scratch_txt`] but with one optional value
Expand All @@ -58,21 +64,30 @@ impl UiBuffer {
}

/// Attempts to clear the buffer if it's over the maximum length allowed.
/// This is to prevent us from making a giant vec over time.
pub fn refresh_buffer(&mut self) {
if self.buffer.len() > self.max_len {
self.buffer.clear();
}
}

/// Pushes a new scratch sheet text, which means it's not handling any clearing at all.
pub fn push(&mut self, txt: impl AsRef<str>) -> *const sys::cty::c_char {
unsafe {
let len = self.buffer.len();
self.buffer.extend(txt.as_ref().as_bytes());
self.buffer.push(b'\0');
/// Given a position, gives an offset from the start of the scatch buffer.
///
/// # Safety
/// This can return a pointer to undefined data if given a `pos >= self.buffer.len()`.
/// This is marked as unsafe to reflect that.
pub unsafe fn offset(&self, pos: usize) -> *const sys::cty::c_char {
self.buffer.as_ptr().add(pos) as *const _
}

self.buffer.as_ptr().add(len) as *const _
}
/// Pushes a new scratch sheet text and return the byte index where the sub-string
/// starts.
pub fn push(&mut self, txt: impl AsRef<str>) -> usize {
let len = self.buffer.len();
self.buffer.extend(txt.as_ref().as_bytes());
self.buffer.push(b'\0');

len
}
}

Expand Down
43 changes: 22 additions & 21 deletions imgui/src/widget/drag.rs
Expand Up @@ -209,22 +209,22 @@ where
/// Returns true if the slider value was changed.
#[doc(alias = "DragFloatRange2")]
pub fn build(self, ui: &Ui, min: &mut f32, max: &mut f32) -> bool {
let label;
let mut display_format = std::ptr::null();
let mut max_display_format = std::ptr::null();

// we do this ourselves the long way...
unsafe {
let buffer = &mut *ui.scratch_buffer().get();
buffer.refresh_buffer();

label = buffer.push(self.label);
if let Some(v) = self.display_format {
display_format = buffer.push(v);
}
if let Some(v) = self.max_display_format {
max_display_format = buffer.push(v);
}
let label_start = buffer.push(self.label);
let display_format = self.display_format.as_ref().map(|v| buffer.push(v));
let max_display_format = self.max_display_format.as_ref().map(|v| buffer.push(v));

let label = buffer.offset(label_start);
let display_format = display_format
.map(|v| buffer.offset(v))
.unwrap_or_else(std::ptr::null);
let max_display_format = max_display_format
.map(|v| buffer.offset(v))
.unwrap_or_else(std::ptr::null);

sys::igDragFloatRange2(
label,
Expand Down Expand Up @@ -253,20 +253,21 @@ where
#[doc(alias = "DragIntRange2")]
pub fn build(self, ui: &Ui, min: &mut i32, max: &mut i32) -> bool {
unsafe {
let mut display_format = std::ptr::null();
let mut max_display_format = std::ptr::null();

// we do this ourselves the long way...
let buffer = &mut *ui.scratch_buffer().get();
buffer.refresh_buffer();

let label = buffer.push(self.label);
if let Some(v) = self.display_format {
display_format = buffer.push(v);
}
if let Some(v) = self.max_display_format {
max_display_format = buffer.push(v);
}
let label_start = buffer.push(self.label);
let display_format = self.display_format.as_ref().map(|v| buffer.push(v));
let max_display_format = self.max_display_format.as_ref().map(|v| buffer.push(v));

let label = buffer.offset(label_start);
let display_format = display_format
.map(|v| buffer.offset(v))
.unwrap_or_else(std::ptr::null);
let max_display_format = max_display_format
.map(|v| buffer.offset(v))
.unwrap_or_else(std::ptr::null);

sys::igDragIntRange2(
label,
Expand Down

0 comments on commit cef617b

Please sign in to comment.