From 8b9ab3d0311dfe2cc52453f429843d2cabe71282 Mon Sep 17 00:00:00 2001 From: kyren Date: Mon, 19 Mar 2018 15:26:21 -0400 Subject: [PATCH] Small renames and comments to better communicate the intention of stack checking functions --- src/function.rs | 9 ++++----- src/lua.rs | 54 ++++++++++++++++++++++++------------------------- src/scope.rs | 6 +++--- src/string.rs | 4 ++-- src/table.rs | 24 +++++++++++----------- src/thread.rs | 12 +++++------ src/userdata.rs | 8 ++++---- src/util.rs | 12 ++++++----- 8 files changed, 65 insertions(+), 64 deletions(-) diff --git a/src/function.rs b/src/function.rs index 5829ed1..baee0f7 100644 --- a/src/function.rs +++ b/src/function.rs @@ -3,8 +3,7 @@ use std::os::raw::c_int; use ffi; use error::{Error, Result}; -use util::{check_stack, check_stack_err, error_traceback, pop_error, protect_lua_closure, - StackGuard}; +use util::{assert_stack, check_stack, error_traceback, pop_error, protect_lua_closure, StackGuard}; use types::LuaRef; use value::{FromLuaMulti, MultiValue, ToLuaMulti}; @@ -69,7 +68,7 @@ impl<'lua> Function<'lua> { let results = unsafe { let _sg = StackGuard::new(lua.state); - check_stack_err(lua.state, nargs + 3)?; + check_stack(lua.state, nargs + 3)?; ffi::lua_pushcfunction(lua.state, error_traceback); let stack_start = ffi::lua_gettop(lua.state); @@ -83,7 +82,7 @@ impl<'lua> Function<'lua> { } let nresults = ffi::lua_gettop(lua.state) - stack_start; let mut results = MultiValue::new(); - check_stack(lua.state, 2); + assert_stack(lua.state, 2); for _ in 0..nresults { results.push_front(lua.pop_value()); } @@ -156,7 +155,7 @@ impl<'lua> Function<'lua> { unsafe { let _sg = StackGuard::new(lua.state); - check_stack_err(lua.state, nargs + 5)?; + check_stack(lua.state, nargs + 5)?; lua.push_ref(&self.0); ffi::lua_pushinteger(lua.state, nargs as ffi::lua_Integer); for arg in args { diff --git a/src/lua.rs b/src/lua.rs index 3d2dd30..b152ee0 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -11,10 +11,10 @@ use libc; use ffi; use error::{Error, Result}; -use util::{callback_error, check_stack, check_stack_err, gc_guard, get_userdata, - get_wrapped_error, init_error_metatables, pop_error, protect_lua, protect_lua_closure, - push_string, push_userdata, push_wrapped_error, safe_pcall, safe_xpcall, - userdata_destructor, StackGuard}; +use util::{assert_stack, callback_error, check_stack, gc_guard, get_userdata, get_wrapped_error, + init_error_metatables, pop_error, protect_lua, protect_lua_closure, push_string, + push_userdata, push_wrapped_error, safe_pcall, safe_xpcall, userdata_destructor, + StackGuard}; use value::{FromLua, FromLuaMulti, MultiValue, Nil, ToLua, ToLuaMulti, Value}; use types::{Callback, Integer, LightUserData, LuaRef, Number, RefType, RegistryKey}; use string::String; @@ -91,7 +91,7 @@ impl Lua { pub fn load(&self, source: &str, name: Option<&str>) -> Result { unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 1); + assert_stack(self.state, 1); match if let Some(name) = name { let name = @@ -155,7 +155,7 @@ impl Lua { pub fn create_string(&self, s: &str) -> Result { unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 4); + assert_stack(self.state, 4); push_string(self.state, s)?; Ok(String(self.pop_ref())) } @@ -165,7 +165,7 @@ impl Lua { pub fn create_table(&self) -> Result { unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 3); + assert_stack(self.state, 3); unsafe extern "C" fn new_table(state: *mut ffi::lua_State) -> c_int { ffi::lua_newtable(state); 1 @@ -186,7 +186,7 @@ impl Lua { let _sg = StackGuard::new(self.state); // `Lua` instance assumes that on any callback, the Lua stack has at least LUA_MINSTACK // slots available to avoid panics. - check_stack_err(self.state, 5 + ffi::LUA_MINSTACK)?; + check_stack(self.state, 5 + ffi::LUA_MINSTACK)?; unsafe extern "C" fn new_table(state: *mut ffi::lua_State) -> c_int { ffi::lua_newtable(state); @@ -310,7 +310,7 @@ impl Lua { pub fn create_thread<'lua>(&'lua self, func: Function<'lua>) -> Result> { unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 2); + assert_stack(self.state, 2); let thread_state = protect_lua_closure(self.state, 0, 1, |state| ffi::lua_newthread(state))?; @@ -333,7 +333,7 @@ impl Lua { pub fn globals(&self) -> Table { unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 2); + assert_stack(self.state, 2); ffi::lua_rawgeti(self.state, ffi::LUA_REGISTRYINDEX, ffi::LUA_RIDX_GLOBALS); Table(self.pop_ref()) } @@ -374,7 +374,7 @@ impl Lua { Value::String(s) => Ok(s), v => unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 4); + assert_stack(self.state, 4); let ty = v.type_name(); self.push_value(v); @@ -402,7 +402,7 @@ impl Lua { Value::Integer(i) => Ok(i), v => unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 2); + assert_stack(self.state, 2); let ty = v.type_name(); self.push_value(v); @@ -430,7 +430,7 @@ impl Lua { Value::Number(n) => Ok(n), v => unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 2); + assert_stack(self.state, 2); let ty = v.type_name(); self.push_value(v); @@ -484,7 +484,7 @@ impl Lua { let t = t.to_lua(self)?; unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 5); + assert_stack(self.state, 5); push_string(self.state, name)?; self.push_value(t); @@ -506,7 +506,7 @@ impl Lua { pub fn named_registry_value<'lua, T: FromLua<'lua>>(&'lua self, name: &str) -> Result { let value = unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 4); + assert_stack(self.state, 4); push_string(self.state, name)?; unsafe extern "C" fn get_registry(state: *mut ffi::lua_State) -> c_int { @@ -537,7 +537,7 @@ impl Lua { let t = t.to_lua(self)?; unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 2); + assert_stack(self.state, 2); self.push_value(t); let registry_id = gc_guard(self.state, || { @@ -564,7 +564,7 @@ impl Lua { } let _sg = StackGuard::new(self.state); - check_stack(self.state, 2); + assert_stack(self.state, 2); ffi::lua_rawgeti( self.state, @@ -825,7 +825,7 @@ impl Lua { } } RefType::Registry { registry_id } => { - check_stack(self.state, 2); + assert_stack(self.state, 2); ffi::lua_rawgeti( self.state, ffi::LUA_REGISTRYINDEX, @@ -900,7 +900,7 @@ impl Lua { } let _sg = StackGuard::new(self.state); - check_stack(self.state, 6); + assert_stack(self.state, 6); let mut methods = UserDataMethods { methods: HashMap::new(), @@ -1025,7 +1025,7 @@ impl Lua { let results = (*func)(&lua, args)?; let nresults = results.len() as c_int; - check_stack_err(state, nresults)?; + check_stack(state, nresults)?; for r in results { lua.push_value(r); } @@ -1036,7 +1036,7 @@ impl Lua { unsafe { let _sg = StackGuard::new(self.state); - check_stack(self.state, 4); + assert_stack(self.state, 4); push_userdata::(self.state, func)?; @@ -1061,7 +1061,7 @@ impl Lua { T: UserData, { let _sg = StackGuard::new(self.state); - check_stack(self.state, 4); + assert_stack(self.state, 4); push_userdata::>(self.state, RefCell::new(data))?; @@ -1168,7 +1168,7 @@ impl Lua { *(ffi::lua_getextraspace(state) as *mut *mut ExtraData) = extra_data; rlua_debug_assert!(ffi::lua_gettop(state) == 0, "stack leak during creation"); - check_stack(state, REF_STACK_SIZE); + assert_stack(state, REF_STACK_SIZE); ffi::lua_settop(state, REF_STACK_SIZE); Lua { @@ -1183,7 +1183,7 @@ impl Lua { // in the callback. fn setup_callback_stack<'lua>(&'lua self) -> Result> { unsafe { - check_stack(self.state, 2); + assert_stack(self.state, 2); let nargs = ffi::lua_gettop(self.state); let stack_nargs = cmp::min(REF_STACK_SIZE, nargs); @@ -1253,12 +1253,12 @@ impl Lua { } if nargs < REF_STACK_SIZE { - check_stack_err(self.state, REF_STACK_SIZE - nargs + ffi::LUA_MINSTACK)?; + check_stack(self.state, REF_STACK_SIZE - nargs + ffi::LUA_MINSTACK)?; ffi::lua_settop(self.state, REF_STACK_SIZE); Ok(args) } else if nargs > REF_STACK_SIZE { if nargs - REF_STACK_SIZE < ffi::LUA_MINSTACK { - check_stack_err(self.state, ffi::LUA_MINSTACK - (nargs - REF_STACK_SIZE))?; + check_stack(self.state, ffi::LUA_MINSTACK - (nargs - REF_STACK_SIZE))?; } // If the total number of arguments exceeds the ref stack area, pop off the rest of @@ -1271,7 +1271,7 @@ impl Lua { extra_args.extend(args.into_vec_rev()); Ok(MultiValue::from_vec_rev(extra_args)) } else { - check_stack_err(self.state, ffi::LUA_MINSTACK)?; + check_stack(self.state, ffi::LUA_MINSTACK)?; Ok(args) } } diff --git a/src/scope.rs b/src/scope.rs index 9084648..7a324d4 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -5,7 +5,7 @@ use std::marker::PhantomData; use ffi; use error::{Error, Result}; -use util::{check_stack, take_userdata, StackGuard}; +use util::{assert_stack, take_userdata, StackGuard}; use value::{FromLuaMulti, ToLuaMulti}; use types::Callback; use lua::Lua; @@ -74,7 +74,7 @@ impl<'scope> Scope<'scope> { destructors.push(Box::new(move || { let state = f_destruct.lua.state; let _sg = StackGuard::new(state); - check_stack(state, 2); + assert_stack(state, 2); f_destruct.lua.push_ref(&f_destruct); ffi::lua_getupvalue(state, -1, 1); @@ -133,7 +133,7 @@ impl<'scope> Scope<'scope> { destructors.push(Box::new(move || { let state = u_destruct.lua.state; let _sg = StackGuard::new(state); - check_stack(state, 1); + assert_stack(state, 1); u_destruct.lua.push_ref(&u_destruct); Box::new(take_userdata::>(state)) })); diff --git a/src/string.rs b/src/string.rs index 835eb54..0f6ded1 100644 --- a/src/string.rs +++ b/src/string.rs @@ -2,7 +2,7 @@ use std::{slice, str}; use ffi; use error::{Error, Result}; -use util::{check_stack, StackGuard}; +use util::{assert_stack, StackGuard}; use types::LuaRef; /// Handle to an internal Lua string. @@ -70,7 +70,7 @@ impl<'lua> String<'lua> { let lua = self.0.lua; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 1); + assert_stack(lua.state, 1); lua.push_ref(&self.0); rlua_debug_assert!( diff --git a/src/table.rs b/src/table.rs index 608edf0..a1f2455 100644 --- a/src/table.rs +++ b/src/table.rs @@ -3,7 +3,7 @@ use std::os::raw::c_int; use ffi; use error::Result; -use util::{check_stack, protect_lua, protect_lua_closure, StackGuard}; +use util::{assert_stack, protect_lua, protect_lua_closure, StackGuard}; use types::{Integer, LuaRef, RefType}; use value::{FromLua, ToLua}; @@ -55,7 +55,7 @@ impl<'lua> Table<'lua> { let value = value.to_lua(lua)?; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 6); + assert_stack(lua.state, 6); lua.push_ref(&self.0); lua.push_value(key); @@ -102,7 +102,7 @@ impl<'lua> Table<'lua> { let key = key.to_lua(lua)?; let value = unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 5); + assert_stack(lua.state, 5); lua.push_ref(&self.0); lua.push_value(key); @@ -124,7 +124,7 @@ impl<'lua> Table<'lua> { unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 5); + assert_stack(lua.state, 5); lua.push_ref(&self.0); lua.push_value(key); @@ -148,7 +148,7 @@ impl<'lua> Table<'lua> { unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 6); + assert_stack(lua.state, 6); lua.push_ref(&self.0); lua.push_value(key); @@ -170,7 +170,7 @@ impl<'lua> Table<'lua> { let key = key.to_lua(lua)?; let value = unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 3); + assert_stack(lua.state, 3); lua.push_ref(&self.0); lua.push_value(key); @@ -189,7 +189,7 @@ impl<'lua> Table<'lua> { let lua = self.0.lua; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 4); + assert_stack(lua.state, 4); lua.push_ref(&self.0); protect_lua_closure(lua.state, 1, 0, |state| ffi::luaL_len(state, -1)) } @@ -200,7 +200,7 @@ impl<'lua> Table<'lua> { let lua = self.0.lua; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 1); + assert_stack(lua.state, 1); lua.push_ref(&self.0); let len = ffi::lua_rawlen(lua.state, -1); len as Integer @@ -214,7 +214,7 @@ impl<'lua> Table<'lua> { let lua = self.0.lua; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 1); + assert_stack(lua.state, 1); lua.push_ref(&self.0); if ffi::lua_getmetatable(lua.state, -1) == 0 { None @@ -233,7 +233,7 @@ impl<'lua> Table<'lua> { let lua = self.0.lua; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 1); + assert_stack(lua.state, 1); lua.push_ref(&self.0); if let Some(metatable) = metatable { lua.push_ref(&metatable.0); @@ -366,7 +366,7 @@ where let res = (|| { let res = unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 6); + assert_stack(lua.state, 6); lua.push_ref(&self.table); lua.push_ref(&next_key); @@ -426,7 +426,7 @@ where let res = unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 5); + assert_stack(lua.state, 5); lua.push_ref(&self.table); match protect_lua_closure(lua.state, 1, 1, |state| ffi::lua_geti(state, -1, index)) diff --git a/src/thread.rs b/src/thread.rs index cefde03..daad7b3 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -2,7 +2,7 @@ use std::os::raw::c_int; use ffi; use error::{Error, Result}; -use util::{check_stack, check_stack_err, error_traceback, pop_error, StackGuard}; +use util::{assert_stack, check_stack, error_traceback, pop_error, StackGuard}; use types::LuaRef; use value::{FromLuaMulti, MultiValue, ToLuaMulti}; @@ -80,7 +80,7 @@ impl<'lua> Thread<'lua> { let args = args.to_lua_multi(lua)?; let results = unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 1); + assert_stack(lua.state, 1); lua.push_ref(&self.0); let thread_state = ffi::lua_tothread(lua.state, -1); @@ -93,8 +93,8 @@ impl<'lua> Thread<'lua> { ffi::lua_pop(lua.state, 1); let nargs = args.len() as c_int; - check_stack_err(lua.state, nargs)?; - check_stack_err(thread_state, nargs + 1)?; + check_stack(lua.state, nargs)?; + check_stack(thread_state, nargs + 1)?; for arg in args { lua.push_value(arg); @@ -111,7 +111,7 @@ impl<'lua> Thread<'lua> { let mut results = MultiValue::new(); ffi::lua_xmove(thread_state, lua.state, nresults); - check_stack(lua.state, 2); + assert_stack(lua.state, 2); for _ in 0..nresults { results.push_front(lua.pop_value()); } @@ -125,7 +125,7 @@ impl<'lua> Thread<'lua> { let lua = self.0.lua; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 1); + assert_stack(lua.state, 1); lua.push_ref(&self.0); let thread_state = ffi::lua_tothread(lua.state, -1); diff --git a/src/userdata.rs b/src/userdata.rs index 94fd3d0..acedb35 100644 --- a/src/userdata.rs +++ b/src/userdata.rs @@ -5,7 +5,7 @@ use std::string::String as StdString; use ffi; use error::{Error, Result}; -use util::{check_stack, get_userdata, StackGuard}; +use util::{assert_stack, get_userdata, StackGuard}; use types::{Callback, LuaRef}; use value::{FromLua, FromLuaMulti, ToLua, ToLuaMulti}; use lua::Lua; @@ -418,7 +418,7 @@ impl<'lua> AnyUserData<'lua> { let v = v.to_lua(lua)?; unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 2); + assert_stack(lua.state, 2); lua.push_ref(&self.0); lua.push_value(v); ffi::lua_setuservalue(lua.state, -2); @@ -433,7 +433,7 @@ impl<'lua> AnyUserData<'lua> { let lua = self.0.lua; let res = unsafe { let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 3); + assert_stack(lua.state, 3); lua.push_ref(&self.0); ffi::lua_getuservalue(lua.state, -1); lua.pop_value() @@ -449,7 +449,7 @@ impl<'lua> AnyUserData<'lua> { unsafe { let lua = self.0.lua; let _sg = StackGuard::new(lua.state); - check_stack(lua.state, 3); + assert_stack(lua.state, 3); lua.push_ref(&self.0); diff --git a/src/util.rs b/src/util.rs index 99b623c..9becb0f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -10,16 +10,18 @@ use error::{Error, Result}; // Checks that Lua has enough free stack space for future stack operations. On failure, this will // panic. -pub unsafe fn check_stack(state: *mut ffi::lua_State, amount: c_int) { +pub unsafe fn assert_stack(state: *mut ffi::lua_State, amount: c_int) { + // TODO: This should only be triggered when there is a logic error in `rlua`. In the future, + // when there is a way to be confident about stack safety and test it, this could be enabled + // only when `cfg!(debug_assertions)` is true. rlua_assert!( ffi::lua_checkstack(state, amount) != 0, "out of stack space" ); } -// Similar to `check_stack`, but returns `Error::StackError` on failure. Useful for user controlled -// sizes, which should not cause a panic. -pub unsafe fn check_stack_err(state: *mut ffi::lua_State, amount: c_int) -> Result<()> { +// Similar to `assert_stack`, but returns `Error::StackError` on failure. +pub unsafe fn check_stack(state: *mut ffi::lua_State, amount: c_int) -> Result<()> { if ffi::lua_checkstack(state, amount) == 0 { Err(Error::StackError) } else { @@ -452,7 +454,7 @@ pub unsafe fn gc_guard R>(state: *mut ffi::lua_State, f: F) -> // Initialize the error, panic, and destructed userdata metatables. pub unsafe fn init_error_metatables(state: *mut ffi::lua_State) { - check_stack(state, 8); + assert_stack(state, 8); // Create error metatable