From 7ab11c5f28403eca91eec687fbf9a0bdbf0a9745 Mon Sep 17 00:00:00 2001 From: Machtan Date: Tue, 9 Feb 2016 19:34:55 +0100 Subject: [PATCH] Remove the integer conversion macros and instead make it into a function named 'validate_int' (and start work on safely mutable rectangles) --- src/sdl2/controller.rs | 8 +++---- src/sdl2/joystick.rs | 15 +++++++------ src/sdl2/macros.rs | 25 --------------------- src/sdl2/rect.rs | 5 +++-- src/sdl2/render.rs | 23 ++++++++++--------- src/sdl2/util.rs | 51 ++++++++++++++++++++++++++++++++++++++++++ src/sdl2/video.rs | 8 +++---- 7 files changed, 82 insertions(+), 53 deletions(-) diff --git a/src/sdl2/controller.rs b/src/sdl2/controller.rs index 85c4f464..03c8a707 100644 --- a/src/sdl2/controller.rs +++ b/src/sdl2/controller.rs @@ -5,7 +5,7 @@ use GameControllerSubsystem; use SdlResult; use get_error; use joystick; -use util::CStringExt; +use util::{CStringExt, validate_int}; use sys::controller as ll; use sys::event::{SDL_QUERY, SDL_ENABLE}; @@ -25,7 +25,7 @@ impl GameControllerSubsystem { /// Return true if the joystick at index `id` is a game controller. #[inline] pub fn is_game_controller(&self, id: u32) -> bool { - match u32_to_int!(id) { + match validate_int(id) { Ok(id) => unsafe { ll::SDL_IsGameController(id) != 0 }, Err(..) => false } @@ -36,7 +36,7 @@ impl GameControllerSubsystem { /// maximum number can be retreived using the `SDL_NumJoysticks` /// function. pub fn open(&self, id: u32) -> SdlResult { - let id = try!(u32_to_int!(id)); + let id = try!(validate_int(id)); let controller = unsafe { ll::SDL_GameControllerOpen(id) }; @@ -52,7 +52,7 @@ impl GameControllerSubsystem { /// Return the name of the controller at index `id` pub fn name_for_index(&self, id: u32) -> SdlResult { - let id = try!(u32_to_int!(id)); + let id = try!(validate_int(id)); let name = unsafe { ll::SDL_GameControllerNameForIndex(id) }; c_str_to_string_or_err(name) diff --git a/src/sdl2/joystick.rs b/src/sdl2/joystick.rs index e999cb44..b80976b8 100644 --- a/src/sdl2/joystick.rs +++ b/src/sdl2/joystick.rs @@ -8,6 +8,7 @@ use sys::event::{SDL_QUERY, SDL_ENABLE}; use std::ffi::{CString, CStr, NulError}; use std::fmt::{Display, Formatter, Error}; use libc::c_char; +use util::validate_int; impl JoystickSubsystem { /// Retreive the total number of attached joysticks *and* controllers identified by SDL. @@ -23,7 +24,7 @@ impl JoystickSubsystem { /// Attempt to open the joystick at number `id` and return it. pub fn open(&self, id: u32) -> SdlResult { - let id = try!(u32_to_int!(id)); + let id = try!(validate_int(id)); let joystick = unsafe { ll::SDL_JoystickOpen(id) }; @@ -39,7 +40,7 @@ impl JoystickSubsystem { /// Return the name of the joystick at index `id` pub fn name_for_index(&self, id: u32) -> SdlResult { - let id = try!(u32_to_int!(id)); + let id = try!(validate_int(id)); let name = unsafe { ll::SDL_JoystickNameForIndex(id) }; c_str_to_string_or_err(name) @@ -47,7 +48,7 @@ impl JoystickSubsystem { /// Get the GUID for the joystick number `id` pub fn device_guid(&self, id: u32) -> SdlResult { - let id = try!(u32_to_int!(id)); + let id = try!(validate_int(id)); let raw = unsafe { ll::SDL_JoystickGetDeviceGUID(id) }; @@ -151,7 +152,7 @@ impl Joystick { // get_error() returns a non-empty string. clear_error(); - let axis = try!(u32_to_int!(axis)); + let axis = try!(validate_int(axis)); let pos = unsafe { ll::SDL_JoystickGetAxis(self.raw, axis) }; if pos != 0 { @@ -187,7 +188,7 @@ impl Joystick { // error... clear_error(); - let button = try!(u32_to_int!(button)); + let button = try!(validate_int(button)); let pressed = unsafe { ll::SDL_JoystickGetButton(self.raw, button) }; match pressed { @@ -225,7 +226,7 @@ impl Joystick { let mut dx = 0; let mut dy = 0; - let ball = try!(u32_to_int!(ball)); + let ball = try!(validate_int(ball)); let result = unsafe { ll::SDL_JoystickGetBall(self.raw, ball, &mut dx, &mut dy) }; if result == 0 { @@ -254,7 +255,7 @@ impl Joystick { // have to use the same hack as `axis`... clear_error(); - let hat = try!(u32_to_int!(hat)); + let hat = try!(validate_int(hat)); let result = unsafe { ll::SDL_JoystickGetHat(self.raw, hat) }; let state = HatState::from_raw(result as u8); diff --git a/src/sdl2/macros.rs b/src/sdl2/macros.rs index 93ad9bb7..fb3be86a 100644 --- a/src/sdl2/macros.rs +++ b/src/sdl2/macros.rs @@ -21,28 +21,3 @@ macro_rules! impl_raw_constructor( )+ ) ); - -/// Many SDL functions will accept `int` values, even if it doesn't make sense for the values to be negative. -/// In the cases that SDL doesn't check negativity, passing negative values could be unsafe. -/// For example, `SDL_JoystickGetButton` uses the index argument to access an array without checking if it's negative, -/// which could potentially lead to segmentation faults. -macro_rules! u32_to_int( - ($value:expr) => ( - if $value >= 1<<31 { Err(format!("`{}` is out of bounds.", stringify!($value))) } - else { Ok($value as ::libc::c_int) } - ) -); - -macro_rules! usize_to_int( - ($value:expr) => ( - if $value >= 1<<31 { Err(format!("`{}` is out of bounds.", stringify!($value))) } - else { Ok($value as ::libc::c_int) } - ) -); - -macro_rules! int_to_u32( - ($value:expr) => ( - if $value < 0 { Err(format!("`{}` is out of bounds.", stringify!($value))) } - else { Ok($value as u32) } - ) -); diff --git a/src/sdl2/rect.rs b/src/sdl2/rect.rs index 13964832..3142b48f 100644 --- a/src/sdl2/rect.rs +++ b/src/sdl2/rect.rs @@ -2,6 +2,7 @@ use sys::rect as ll; use std::mem; use std::ptr; use std::ops::{BitAnd, BitOr}; +use util::validate_int; use SdlResult; use ErrorMessage; @@ -121,8 +122,8 @@ impl Rect { /// If `width` or `height` is zero, `Ok(None)` is returned. /// If the arguments violate any of the other rectangle invariants, an error is returned. pub fn new(x: i32, y: i32, width: u32, height: u32) -> SdlResult> { - let width = try!(u32_to_int!(width)); - let height = try!(u32_to_int!(height)); + let width = try!(validate_int(width)); + let height = try!(validate_int(height)); if x.checked_add(width).is_none() { Err(ErrorMessage("`x` + `width` overflows.".into())) diff --git a/src/sdl2/render.rs b/src/sdl2/render.rs index 67b91ffa..460c388e 100644 --- a/src/sdl2/render.rs +++ b/src/sdl2/render.rs @@ -46,6 +46,7 @@ use std::ffi::CStr; use num::FromPrimitive; use std::vec::Vec; use std::rc::Rc; +use util::validate_int; use sys::render as ll; @@ -173,7 +174,7 @@ impl RendererBuilder { pub fn build(self) -> SdlResult> { let index = match self.index { None => -1, - Some(index) => try!(u32_to_int!(index)) + Some(index) => try!(validate_int(index)) }; let raw = unsafe { ll::SDL_CreateRenderer(self.window.raw(), index, self.renderer_flags) @@ -332,8 +333,8 @@ impl<'a> Renderer<'a> { /// /// `size` is the width and height of the texture. pub fn create_texture(&self, format: pixels::PixelFormatEnum, access: TextureAccess, (width, height): (u32, u32)) -> SdlResult { - let width = try!(u32_to_int!(width)); - let height = try!(u32_to_int!(height)); + let width = try!(validate_int(width)); + let height = try!(validate_int(height)); // If the pixel format is YUV 4:2:0 and planar, the width and height must // be multiples-of-two. See issue #334 for details. @@ -478,8 +479,8 @@ impl<'a> Renderer<'a> { /// Sets a device independent resolution for rendering. pub fn set_logical_size(&mut self, width: u32, height: u32) -> SdlResult<()> { - let width = try!(u32_to_int!(width)); - let height = try!(u32_to_int!(height)); + let width = try!(validate_int(width)); + let height = try!(validate_int(height)); let result = unsafe { ll::SDL_RenderSetLogicalSize(self.raw, width, height) }; match result { 0 => Ok(()), @@ -837,8 +838,8 @@ impl<'renderer> RenderTarget<'renderer> { /// /// The old render target is returned if the function is successful. pub fn create_and_set(&mut self, format: pixels::PixelFormatEnum, (width, height): (u32, u32)) -> SdlResult> { - let width = try!(u32_to_int!(width)); - let height = try!(u32_to_int!(height)); + let width = try!(validate_int(width)); + let height = try!(validate_int(height)); let new_texture_raw = unsafe { let access = ll::SDL_TEXTUREACCESS_TARGET; @@ -1043,7 +1044,7 @@ impl Texture { } } - let pitch = try!(usize_to_int!(pitch)); + let pitch = try!(validate_int(pitch as u32)); ll::SDL_UpdateTexture(self.raw, rect_raw_ptr, pixel_data.as_ptr() as *const _, pitch) }; @@ -1103,9 +1104,9 @@ impl Texture { return Err(ErrorMessage("One or more of the plane lengths is not correct (should be pitch * height).".into())); } - let y_pitch = try!(usize_to_int!(y_pitch)); - let u_pitch = try!(usize_to_int!(u_pitch)); - let v_pitch = try!(usize_to_int!(v_pitch)); + let y_pitch = try!(validate_int(y_pitch as u32)); + let u_pitch = try!(validate_int(u_pitch as u32)); + let v_pitch = try!(validate_int(v_pitch as u32)); unsafe { let result = ll::SDL_UpdateYUVTexture( diff --git a/src/sdl2/util.rs b/src/sdl2/util.rs index e50374e3..966a3db3 100644 --- a/src/sdl2/util.rs +++ b/src/sdl2/util.rs @@ -1,8 +1,59 @@ use std::ffi::{CString, NulError}; +use std::ops::Add; use SdlResult; use ErrorMessage; +/// Validates and converts the given u32 to a positive C integer. +pub fn validate_int(value: u32) -> Result<::libc::c_int, String> { + // Many SDL functions will accept `int` values, even if it doesn't make sense + // for the values to be negative. + // In the cases that SDL doesn't check negativity, passing negative values + // could be unsafe. + // For example, `SDL_JoystickGetButton` uses the index argument to access an + // array without checking if it's negative, which could potentially lead to + // segmentation faults. + if value >= 1 << 31 { + Err(format!("`{}` is out of bounds.", value)) + } else { + Ok(value as ::libc::c_int) + } +} + +pub struct CheckedInteger { + value: u32, +} +impl CheckedInteger { + pub fn new(value: u32) -> Result { + if value >= 1 << 31 { + Err(format!("The value '{}' is too big for a C int.", value)) + } else { + Ok(CheckedInteger { value: value } ) + } + } + + pub fn add(&self, value: u32) -> Result { + if let Some(new) = self.value.checked_add(value) { + CheckedInteger::new(new) + } else { + Err("The combined value overflowed".to_owned()) + } + } + + pub fn sub(&self, value: u32) -> Result { + if let Some(new) = self.value.checked_add(value) { + CheckedInteger::new(new) + } else { + Err("The combined value underflowed".to_owned()) + } + } + + pub fn value(&self) -> u32 { + self.value + } +} + + pub trait CStringExt { /// Returns an SDL error if the string contains a nul byte. fn unwrap_or_sdlresult(self) -> SdlResult; diff --git a/src/sdl2/video.rs b/src/sdl2/video.rs index f5d37212..1e26428f 100644 --- a/src/sdl2/video.rs +++ b/src/sdl2/video.rs @@ -14,7 +14,7 @@ use EventPump; use SdlResult; use ErrorMessage; use num::FromPrimitive; -use util::CStringExt; +use util::{CStringExt, validate_int}; use get_error; @@ -972,7 +972,7 @@ impl WindowRef { } pub fn set_size(&mut self, w: u32, h: u32) { - match (u32_to_int!(w), u32_to_int!(h)) { + match (validate_int(w), validate_int(h)) { (Ok(w), Ok(h)) => unsafe { ll::SDL_SetWindowSize(self.raw(), w, h) }, _ => () // silently fail (`SDL_SetWindowSize` returns void) } @@ -993,7 +993,7 @@ impl WindowRef { } pub fn set_minimum_size(&mut self, w: u32, h: u32) { - match (u32_to_int!(w), u32_to_int!(h)) { + match (validate_int(w), validate_int(h)) { (Ok(w), Ok(h)) => unsafe { ll::SDL_SetWindowMinimumSize(self.raw(), w, h) }, _ => () // silently fail (`SDL_SetWindowMinimumSize` returns void) } @@ -1007,7 +1007,7 @@ impl WindowRef { } pub fn set_maximum_size(&mut self, w: u32, h: u32) { - match (u32_to_int!(w), u32_to_int!(h)) { + match (validate_int(w), validate_int(h)) { (Ok(w), Ok(h)) => unsafe { ll::SDL_SetWindowMaximumSize(self.raw(), w, h) }, _ => () // silently fail (`SDL_SetWindowMaximumSize` returns void) }