From 8ac78c4585a026d5256186341cb2af79558b7164 Mon Sep 17 00:00:00 2001 From: kyren Date: Thu, 1 Mar 2018 17:14:07 -0500 Subject: [PATCH] Make some changes whose necessity became recently apparent while reading rustc 1.24.1 change notes. So, despite staring intently at the params structure magic in protect_lua_call, there is still a nasty bug. In the event of an error, the return value of the parameters structure could be dropped despite being mem::unintialized. Of course, the actual return values are incidentally always Copy I think, so this wasn't an actual bug, but I've proven to myself the danger of such dark majyyks. Just use Option and be done with it, it doesn't have to be so complicated! Also document why there are a slew of random functions in the ffi module. --- src/ffi.rs | 2 ++ src/util.rs | 29 +++++++---------------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/ffi.rs b/src/ffi.rs index df3d394..eaa40ae 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -200,6 +200,8 @@ extern "C" { pub fn luaL_len(push_state: *mut lua_State, index: c_int) -> lua_Integer; } +// The following are re-implementations of what are macros in the Lua C API + pub unsafe fn lua_getextraspace(state: *mut lua_State) -> *mut c_void { (state as *mut c_void).offset(-(mem::size_of::<*mut c_void>() as isize)) } diff --git a/src/util.rs b/src/util.rs index 647efeb..10dcac7 100644 --- a/src/util.rs +++ b/src/util.rs @@ -114,8 +114,8 @@ where F: FnOnce(*mut ffi::lua_State) -> R, { struct Params { - function: F, - result: R, + function: Option, + result: Option, nresults: c_int, } @@ -126,9 +126,8 @@ where let params = ffi::lua_touserdata(state, -1) as *mut Params; ffi::lua_pop(state, 1); - let function = mem::replace(&mut (*params).function, mem::uninitialized()); - ptr::write(&mut (*params).result, function(state)); - // params now has function uninitialied and result initialized + let function = (*params).function.take().unwrap(); + (*params).result = Some(function(state)); if (*params).nresults == ffi::LUA_MULTRET { ffi::lua_gettop(state) @@ -143,32 +142,18 @@ where ffi::lua_pushcfunction(state, do_call::); ffi::lua_rotate(state, stack_start + 1, 2); - // We are about to do some really scary stuff with the Params structure, both because - // protect_lua_call is very hot, and becuase we would like to allow the function type to be - // FnOnce rather than FnMut. We are using Params here both to pass data to the callback and - // return data from the callback. - // - // params starts out with function initialized and result uninitialized, nresults is Copy so we - // don't care about it. let mut params = Params { - function: f, - result: mem::uninitialized(), + function: Some(f), + result: None, nresults, }; ffi::lua_pushlightuserdata(state, &mut params as *mut Params as *mut c_void); - let ret = ffi::lua_pcall(state, nargs + 1, nresults, stack_start + 1); - let result = mem::replace(&mut params.result, mem::uninitialized()); - - // params now has both function and result uninitialized, so we need to forget it so Drop isn't - // run. - mem::forget(params); - ffi::lua_remove(state, stack_start + 1); if ret == ffi::LUA_OK { - Ok(result) + Ok(params.result.take().unwrap()) } else { Err(pop_error(state, ret)) }