Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unsafety in text fix #746

Merged
merged 2 commits into from Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions imgui-examples/examples/custom_textures.rs
Expand Up @@ -189,8 +189,8 @@ impl Lenna {
decoder.read_image(&mut image)?;
let raw = RawImage2d {
data: Cow::Owned(image),
width: width as u32,
height: height as u32,
width,
height,
format: ClientFormat::U8U8U8,
};
let gl_texture = Texture2d::new(gl_ctx, raw)?;
Expand Down
4 changes: 2 additions & 2 deletions imgui-examples/examples/test_window_impl.rs
Expand Up @@ -759,7 +759,7 @@ CTRL+click on individual component to input value.\n",
);

state.filter.draw();
let lines = vec!["aaa1.c", "bbb1.c", "ccc1.c", "aaa2.cpp", "bbb2.cpp", "ccc2.cpp", "abc.h", "hello, world!"];
let lines = ["aaa1.c", "bbb1.c", "ccc1.c", "aaa2.cpp", "bbb2.cpp", "ccc2.cpp", "abc.h", "hello, world!"];

ui.same_line();
if ui.button("Clear##clear_filter") {
Expand Down Expand Up @@ -1285,7 +1285,7 @@ fn show_app_log(ui: &Ui, app_log: &mut Vec<String>) {
}
ui.same_line();
if ui.button("Copy") {
ui.set_clipboard_text(&ImString::from(app_log.join("\n")));
ui.set_clipboard_text(ImString::from(app_log.join("\n")));
}
ui.separator();
ui.child_window("logwindow")
Expand Down
4 changes: 2 additions & 2 deletions imgui-examples/examples/text_callbacks.rs
Expand Up @@ -106,7 +106,7 @@ fn main() {
if !data.str().is_empty() {
data.remove_chars(0, 1);

if let Some((idx, _)) = data.str().char_indices().rev().next() {
if let Some((idx, _)) = data.str().char_indices().next_back() {
data.remove_chars(idx, 1);
}
}
Expand All @@ -118,7 +118,7 @@ fn main() {
}

// insert last char
if let Some((idx, _)) = self.1.char_indices().rev().next() {
if let Some((idx, _)) = self.1.char_indices().next_back() {
data.push_str(&self.1[idx..]);
}
}
Expand Down
2 changes: 1 addition & 1 deletion imgui-glow-renderer/src/lib.rs
Expand Up @@ -1152,7 +1152,7 @@ fn calculate_matrix(draw_data: &DrawData, clip_origin_is_lower_left: bool) -> [f
}

unsafe fn to_byte_slice<T>(slice: &[T]) -> &[u8] {
std::slice::from_raw_parts(slice.as_ptr().cast(), slice.len() * size_of::<T>())
std::slice::from_raw_parts(slice.as_ptr().cast(), std::mem::size_of_val(slice))
}

const fn imgui_index_type_as_gl() -> u32 {
Expand Down
2 changes: 1 addition & 1 deletion imgui/src/context.rs
Expand Up @@ -523,7 +523,7 @@ impl Context {
// we take this with an `&mut Self` here, which means
// that we can't get the sharedfontatlas through safe code
// otherwise
unsafe { &mut *(self.io_mut().fonts as *mut FontAtlas) }
unsafe { &mut *self.io_mut().fonts }
}

/// Attempts to clone the interior shared font atlas **if it exists**.
Expand Down
2 changes: 0 additions & 2 deletions imgui/src/draw_list.rs
Expand Up @@ -27,7 +27,6 @@ bitflags!(
/// Options for some DrawList operations.
#[repr(C)]
pub struct DrawFlags: u32 {
const NONE = sys::ImDrawFlags_None;
const CLOSED = sys::ImDrawFlags_Closed;
const ROUND_CORNERS_TOP_LEFT = sys::ImDrawFlags_RoundCornersTopLeft;
const ROUND_CORNERS_TOP_RIGHT = sys::ImDrawFlags_RoundCornersTopRight;
Expand All @@ -46,7 +45,6 @@ bitflags!(
/// Draw list flags
#[repr(C)]
pub struct DrawListFlags: u32 {
const NONE = sys::ImDrawListFlags_None;
/// Enable anti-aliased lines/borders (*2 the number of triangles for 1.0f wide line or lines
/// thin enough to be drawn using textures, otherwise *3 the number of triangles)
const ANTI_ALIASED_LINES = sys::ImDrawListFlags_AntiAliasedLines;
Expand Down
11 changes: 7 additions & 4 deletions imgui/src/fonts/glyph_ranges.rs
Expand Up @@ -17,10 +17,6 @@ enum FontGlyphRangeData {
#[derive(Clone, Eq, PartialEq, Debug)]
pub struct FontGlyphRanges(FontGlyphRangeData);
impl FontGlyphRanges {
/// The default set of glyph ranges used by imgui.
pub fn default() -> FontGlyphRanges {
FontGlyphRanges(FontGlyphRangeData::Default)
}
/// A set of glyph ranges appropriate for use with simplified common Chinese text.
pub fn chinese_simplified_common() -> FontGlyphRanges {
FontGlyphRanges(FontGlyphRangeData::ChineseSimplifiedCommon)
Expand Down Expand Up @@ -162,3 +158,10 @@ impl FontGlyphRanges {
}
}
}

impl Default for FontGlyphRanges {
/// The default set of glyph ranges used by imgui.
fn default() -> Self {
FontGlyphRanges(FontGlyphRangeData::Default)
}
}
4 changes: 2 additions & 2 deletions imgui/src/internal.rs
@@ -1,6 +1,6 @@
//! Internal raw utilities (don't use unless you know what you're doing!)

use std::{mem::size_of, slice};
use std::slice;

/// A generic version of the raw imgui-sys ImVector struct types
#[repr(C)]
Expand All @@ -25,7 +25,7 @@ impl<T> ImVector<T> {
unsafe {
sys::igMemFree(self.data as *mut _);

let buffer_ptr = sys::igMemAlloc(size_of::<T>() * data.len()) as *mut T;
let buffer_ptr = sys::igMemAlloc(std::mem::size_of_val(data)) as *mut T;
buffer_ptr.copy_from_nonoverlapping(data.as_ptr(), data.len());

self.size = data.len() as i32;
Expand Down
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
4 changes: 2 additions & 2 deletions imgui/src/render/draw_data.rs
Expand Up @@ -334,7 +334,7 @@ impl From<&DrawData> for OwnedDrawData {
OwnedDrawData {
draw_data: unsafe {
let other_ptr = value.raw();
let mut result = sys::ImDrawData_ImDrawData();
let result = sys::ImDrawData_ImDrawData();
(*result).Valid = other_ptr.Valid;
(*result).TotalIdxCount = other_ptr.TotalIdxCount;
(*result).TotalVtxCount = other_ptr.TotalVtxCount;
Expand Down Expand Up @@ -404,7 +404,7 @@ fn test_owneddrawdata_from_drawdata() {
DisplaySize: sys::ImVec2 { x: 789.0, y: 012.0 },
FramebufferScale: sys::ImVec2 { x: 3.0, y: 7.0 },
#[cfg(feature = "docking")]
OwnerViewport: unsafe { (std::ptr::null_mut() as *mut sys::ImGuiViewport).offset(123) },
OwnerViewport: unsafe { std::ptr::null_mut::<sys::ImGuiViewport>().offset(123) },
};
let draw_data = unsafe { DrawData::from_raw(&draw_data_raw) };

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