Skip to content

Commit 2bd7a2e

Browse files
kyrenkyren
committed
Reduce error_guard code to as little as possible
Also ensure that on error in error_guard the stack is in a predictable place.
1 parent 36134e6 commit 2bd7a2e

File tree

3 files changed

+67
-74
lines changed

3 files changed

+67
-74
lines changed

‎src/lua.rs

+48-53
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,13 @@ impl<'lua> LuaTable<'lua> {
223223
let key = key.to_lua(lua)?;
224224
let value = value.to_lua(lua)?;
225225
unsafe {
226-
check_stack(lua.state, 3);
226+
check_stack(lua.state, 5);
227227
lua.push_ref(lua.state, &self.0);
228228
lua.push_value(lua.state, key);
229229
lua.push_value(lua.state, value);
230-
error_guard(lua.state, 3, 0, |state| {
230+
error_guard(lua.state, 3, |state| {
231231
ffi::lua_settable(state, -3);
232+
ffi::lua_pop(state, 1);
232233
Ok(())
233234
})
234235
}
@@ -243,15 +244,15 @@ impl<'lua> LuaTable<'lua> {
243244
let lua = self.0.lua;
244245
let key = key.to_lua(lua)?;
245246
unsafe {
246-
check_stack(lua.state, 2);
247+
check_stack(lua.state, 4);
247248
lua.push_ref(lua.state, &self.0);
248249
lua.push_value(lua.state, key.to_lua(lua)?);
249-
let res = error_guard(lua.state, 2, 0, |state| {
250+
error_guard(lua.state, 2, |state| {
250251
ffi::lua_gettable(state, -2);
251-
let res = lua.pop_value(state);
252-
ffi::lua_pop(state, 1);
253-
Ok(res)
252+
Ok(())
254253
})?;
254+
let res = lua.pop_value(lua.state);
255+
ffi::lua_pop(lua.state, 1);
255256
V::from_lua(res, lua)
256257
}
257258
}
@@ -261,15 +262,16 @@ impl<'lua> LuaTable<'lua> {
261262
let lua = self.0.lua;
262263
let key = key.to_lua(lua)?;
263264
unsafe {
264-
check_stack(lua.state, 2);
265+
check_stack(lua.state, 4);
265266
lua.push_ref(lua.state, &self.0);
266267
lua.push_value(lua.state, key);
267-
error_guard(lua.state, 2, 0, |state| {
268+
error_guard(lua.state, 2, |state| {
268269
ffi::lua_gettable(state, -2);
269-
let has = ffi::lua_isnil(state, -1) == 0;
270-
ffi::lua_pop(state, 2);
271-
Ok(has)
272-
})
270+
Ok(())
271+
})?;
272+
let has = ffi::lua_isnil(lua.state, -1) == 0;
273+
ffi::lua_pop(lua.state, 2);
274+
Ok(has)
273275
}
274276
}
275277

@@ -311,10 +313,12 @@ impl<'lua> LuaTable<'lua> {
311313
pub fn len(&self) -> LuaResult<LuaInteger> {
312314
let lua = self.0.lua;
313315
unsafe {
314-
error_guard(lua.state, 0, 0, |state| {
315-
check_stack(state, 1);
316-
lua.push_ref(state, &self.0);
317-
Ok(ffi::luaL_len(state, -1))
316+
check_stack(lua.state, 3);
317+
lua.push_ref(lua.state, &self.0);
318+
error_guard(lua.state, 1, |state| {
319+
let len = ffi::luaL_len(state, -1);
320+
ffi::lua_pop(state, 1);
321+
Ok(len)
318322
})
319323
}
320324
}
@@ -381,31 +385,29 @@ where
381385
let lua = self.table.lua;
382386

383387
unsafe {
384-
check_stack(lua.state, 4);
388+
check_stack(lua.state, 6);
385389

386390
lua.push_ref(lua.state, &self.table);
387391
lua.push_ref(lua.state, &next_key);
388392

389-
match error_guard(lua.state, 2, 0, |state| if ffi::lua_next(state, -2) != 0 {
390-
ffi::lua_pushvalue(state, -2);
391-
let key = lua.pop_value(state);
392-
let value = lua.pop_value(state);
393-
let next_key = lua.pop_ref(lua.state);
394-
ffi::lua_pop(lua.state, 1);
395-
Ok(Some((key, value, next_key)))
396-
} else {
397-
ffi::lua_pop(lua.state, 1);
398-
Ok(None)
399-
}) {
400-
Ok(Some((key, value, next_key))) => {
401-
self.next_key = Some(next_key);
393+
match error_guard(lua.state, 2, |state| Ok(ffi::lua_next(state, -2) != 0)) {
394+
Ok(true) => {
395+
ffi::lua_pushvalue(lua.state, -2);
396+
let key = lua.pop_value(lua.state);
397+
let value = lua.pop_value(lua.state);
398+
self.next_key = Some(lua.pop_ref(lua.state));
399+
ffi::lua_pop(lua.state, 1);
400+
402401
Some((|| {
403-
let key = K::from_lua(key, lua)?;
404-
let value = V::from_lua(value, lua)?;
405-
Ok((key, value))
406-
})())
402+
let key = K::from_lua(key, lua)?;
403+
let value = V::from_lua(value, lua)?;
404+
Ok((key, value))
405+
})())
406+
}
407+
Ok(false) => {
408+
ffi::lua_pop(lua.state, 1);
409+
None
407410
}
408-
Ok(None) => None,
409411
Err(e) => Some(Err(e)),
410412
}
411413
}
@@ -436,27 +438,20 @@ where
436438
let lua = self.table.lua;
437439

438440
unsafe {
439-
check_stack(lua.state, 2);
441+
check_stack(lua.state, 4);
440442

441443
lua.push_ref(lua.state, &self.table);
442-
match error_guard(
443-
lua.state,
444-
1,
445-
0,
446-
|state| if ffi::lua_geti(state, -1, index) != ffi::LUA_TNIL {
447-
let value = lua.pop_value(state);
448-
ffi::lua_pop(state, 1);
449-
Ok(Some(value))
450-
} else {
451-
ffi::lua_pop(state, 2);
452-
Ok(None)
453-
},
454-
) {
455-
Ok(Some(r)) => {
444+
match error_guard(lua.state, 1, |state| Ok(ffi::lua_geti(state, -1, index) != ffi::LUA_TNIL)) {
445+
Ok(true) => {
446+
let value = lua.pop_value(lua.state);
447+
ffi::lua_pop(lua.state, 1);
456448
self.index = Some(index + 1);
457-
Some(V::from_lua(r, lua))
449+
Some(V::from_lua(value, lua))
458450
}
459-
Ok(None) => None,
451+
Ok(false) => {
452+
ffi::lua_pop(lua.state, 2);
453+
None
454+
},
460455
Err(e) => Some(Err(e)),
461456
}
462457
}

‎src/tests.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -810,16 +810,10 @@ fn test_expired_userdata() {
810810
id: u8,
811811
}
812812

813-
impl Drop for Userdata {
814-
fn drop(&mut self) {
815-
println!("dropping {}", self.id);
816-
}
817-
}
818-
819813
impl LuaUserDataType for Userdata {
820814
fn add_methods(methods: &mut LuaUserDataMethods<Self>) {
821815
methods.add_method("access", |lua, this, _| {
822-
println!("accessing userdata {}", this.id);
816+
assert!(this.id == 123);
823817
lua.pack(())
824818
});
825819
}

‎src/util.rs

+18-14
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ macro_rules! cstr {
2020
macro_rules! lua_panic {
2121
($state:expr) => {
2222
{
23-
$crate::ffi::lua_settop($state, 0);
23+
$crate::ffi::lua_settor($state, 0);
2424
panic!("rlua internal error");
2525
}
2626
};
@@ -151,16 +151,15 @@ where
151151
res
152152
}
153153

154-
// Call the given rust function in a protected lua context, similar to pcall.
155-
// The stack given to the protected function is a separate protected stack. This
156-
// catches all calls to lua_error, but ffi functions that can call lua_error are
157-
// still longjmps, and have all the same dangers as longjmps, so extreme care
158-
// must still be taken in code that uses this function. Does not call
159-
// lua_checkstack, and uses 2 extra stack spaces.
154+
// Call the given rust function in a protected lua context, similar to pcall. The stack given to
155+
// the protected function is a separate protected stack. This catches all calls to lua_error, but
156+
// ffi functions that can call lua_error are still longjmps, and have all the same dangers as
157+
// longjmps, so extreme care must still be taken in code that uses this function. Does not call
158+
// lua_checkstack, and uses 2 extra stack spaces. On error, the stack position is set to just below
159+
// the given arguments.
160160
pub unsafe fn error_guard<F, R>(
161161
state: *mut ffi::lua_State,
162162
nargs: c_int,
163-
nresults: c_int,
164163
func: F,
165164
) -> LuaResult<R>
166165
where
@@ -179,7 +178,6 @@ where
179178
unsafe fn cpcall<F>(
180179
state: *mut ffi::lua_State,
181180
nargs: c_int,
182-
nresults: c_int,
183181
mut func: F,
184182
) -> LuaResult<()>
185183
where
@@ -189,15 +187,21 @@ where
189187
ffi::lua_insert(state, -(nargs + 1));
190188
ffi::lua_pushlightuserdata(state, &mut func as *mut F as *mut c_void);
191189
mem::forget(func);
192-
handle_error(state, pcall_with_traceback(state, nargs + 1, nresults))
190+
handle_error(state, pcall_with_traceback(state, nargs + 1, ffi::LUA_MULTRET))
193191
}
194192

195193
let mut res = None;
196-
cpcall(state, nargs, nresults, |state| {
194+
let top = ffi::lua_gettop(state);
195+
if let Err(err) = cpcall(state, nargs, |state| {
197196
res = Some(callback_error(state, || func(state)));
198-
ffi::lua_gettop(state)
199-
})?;
200-
Ok(res.unwrap())
197+
let c = ffi::lua_gettop(state);
198+
c
199+
}) {
200+
ffi::lua_settop(state, top - nargs);
201+
Err(err)
202+
} else {
203+
Ok(res.unwrap())
204+
}
201205
}
202206

203207
// If the return code indicates an error, pops the error off of the stack and

0 commit comments

Comments
 (0)