From 377e9e7907eb054217bdd432bd8f4a35e76c8226 Mon Sep 17 00:00:00 2001 From: Jonathan Spira Date: Tue, 12 Sep 2023 13:15:34 -0400 Subject: [PATCH 1/2] unsafety in text fix --- imgui/src/lib.rs | 11 ++++++++-- imgui/src/string.rs | 35 ++++++++++++++++++++++---------- imgui/src/widget/drag.rs | 43 ++++++++++++++++++++-------------------- 3 files changed, 56 insertions(+), 33 deletions(-) diff --git a/imgui/src/lib.rs b/imgui/src/lib.rs index f250f706f..9eb2353e7 100644 --- a/imgui/src/lib.rs +++ b/imgui/src/lib.rs @@ -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 = 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) }; diff --git a/imgui/src/string.rs b/imgui/src/string.rs index 601682a8e..f9a08f828 100644 --- a/imgui/src/string.rs +++ b/imgui/src/string.rs @@ -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) -> *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. @@ -42,7 +44,11 @@ impl UiBuffer { txt_1: impl AsRef, ) -> (*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 @@ -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) -> *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) -> usize { + let len = self.buffer.len(); + self.buffer.extend(txt.as_ref().as_bytes()); + self.buffer.push(b'\0'); + + len } } diff --git a/imgui/src/widget/drag.rs b/imgui/src/widget/drag.rs index 1c550b87e..bbccebe5b 100644 --- a/imgui/src/widget/drag.rs +++ b/imgui/src/widget/drag.rs @@ -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, @@ -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, From a674d1f5be56b580b2fcc5b7d79b2f9633e7ecd6 Mon Sep 17 00:00:00 2001 From: Jonathan Spira Date: Tue, 12 Sep 2023 13:24:19 -0400 Subject: [PATCH 2/2] clippy don't be so mad! --- imgui-examples/examples/custom_textures.rs | 4 ++-- imgui-examples/examples/test_window_impl.rs | 4 ++-- imgui-examples/examples/text_callbacks.rs | 4 ++-- imgui-glow-renderer/src/lib.rs | 2 +- imgui/src/context.rs | 2 +- imgui/src/draw_list.rs | 2 -- imgui/src/fonts/glyph_ranges.rs | 11 +++++++---- imgui/src/internal.rs | 4 ++-- imgui/src/render/draw_data.rs | 4 ++-- 9 files changed, 19 insertions(+), 18 deletions(-) diff --git a/imgui-examples/examples/custom_textures.rs b/imgui-examples/examples/custom_textures.rs index 6b8cfc575..fabd8a802 100644 --- a/imgui-examples/examples/custom_textures.rs +++ b/imgui-examples/examples/custom_textures.rs @@ -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)?; diff --git a/imgui-examples/examples/test_window_impl.rs b/imgui-examples/examples/test_window_impl.rs index ee9e2c04e..60523c116 100644 --- a/imgui-examples/examples/test_window_impl.rs +++ b/imgui-examples/examples/test_window_impl.rs @@ -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") { @@ -1285,7 +1285,7 @@ fn show_app_log(ui: &Ui, app_log: &mut Vec) { } 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") diff --git a/imgui-examples/examples/text_callbacks.rs b/imgui-examples/examples/text_callbacks.rs index dce2c7efc..a8bd7c512 100644 --- a/imgui-examples/examples/text_callbacks.rs +++ b/imgui-examples/examples/text_callbacks.rs @@ -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); } } @@ -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..]); } } diff --git a/imgui-glow-renderer/src/lib.rs b/imgui-glow-renderer/src/lib.rs index e178fa693..4b70cc662 100644 --- a/imgui-glow-renderer/src/lib.rs +++ b/imgui-glow-renderer/src/lib.rs @@ -1152,7 +1152,7 @@ fn calculate_matrix(draw_data: &DrawData, clip_origin_is_lower_left: bool) -> [f } unsafe fn to_byte_slice(slice: &[T]) -> &[u8] { - std::slice::from_raw_parts(slice.as_ptr().cast(), slice.len() * size_of::()) + std::slice::from_raw_parts(slice.as_ptr().cast(), std::mem::size_of_val(slice)) } const fn imgui_index_type_as_gl() -> u32 { diff --git a/imgui/src/context.rs b/imgui/src/context.rs index 67bb7866b..e30835048 100644 --- a/imgui/src/context.rs +++ b/imgui/src/context.rs @@ -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**. diff --git a/imgui/src/draw_list.rs b/imgui/src/draw_list.rs index 3ca6eeebd..863471345 100644 --- a/imgui/src/draw_list.rs +++ b/imgui/src/draw_list.rs @@ -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; @@ -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; diff --git a/imgui/src/fonts/glyph_ranges.rs b/imgui/src/fonts/glyph_ranges.rs index b22ae5067..29ae59ac8 100644 --- a/imgui/src/fonts/glyph_ranges.rs +++ b/imgui/src/fonts/glyph_ranges.rs @@ -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) @@ -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) + } +} diff --git a/imgui/src/internal.rs b/imgui/src/internal.rs index d6a38a739..ad68764e8 100644 --- a/imgui/src/internal.rs +++ b/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)] @@ -25,7 +25,7 @@ impl ImVector { unsafe { sys::igMemFree(self.data as *mut _); - let buffer_ptr = sys::igMemAlloc(size_of::() * 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; diff --git a/imgui/src/render/draw_data.rs b/imgui/src/render/draw_data.rs index 14fc78977..c93deb95d 100644 --- a/imgui/src/render/draw_data.rs +++ b/imgui/src/render/draw_data.rs @@ -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; @@ -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::().offset(123) }, }; let draw_data = unsafe { DrawData::from_raw(&draw_data_raw) };