Skip to content

Commit

Permalink
Fix a number of instances of memory leakage and add test coverage.
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremyong committed Jan 27, 2014
1 parent 8924e74 commit 1b059b5
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 78 deletions.
4 changes: 3 additions & 1 deletion include/Selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Selector {
_get();
detail::_push_n(_state._l, std::forward<Args>(args)...);
lua_call(_state._l, sizeof...(Args), sizeof...(Ret));
return detail::_pop_n<Ret...>(_state._l);
return detail::_pop_n_reset<Ret...>(_state._l);
}

template <typename T>
Expand All @@ -73,6 +73,7 @@ class Selector {
detail::_push(_state._l, t);
};
_put(push);
lua_settop(_state._l, 0);
}

template <typename T, typename... Funs>
Expand All @@ -83,6 +84,7 @@ class Selector {
_state.Register(t, fun_tuple);
};
_put(push);
lua_settop(_state._l, 0);
}

template <typename Ret, typename... Args>
Expand Down
9 changes: 4 additions & 5 deletions include/State.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class State {
State(State &&other);
~State();

int Size() const {
return lua_gettop(_l);
}

bool Load(const std::string &file);

void Push() {} // Base case
Expand All @@ -61,11 +65,6 @@ class State {
return result;
}
public:
template <typename... T>
typename detail::_pop_n_impl<sizeof...(T), T...>::type Pop() {
return detail::_pop_n<T...>(_l);
}

template <typename Ret, typename... Args>
void Register(std::function<Ret(Args...)> fun) {
constexpr int arity = detail::_arity<Ret>::value;
Expand Down
51 changes: 51 additions & 0 deletions include/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ struct _indices_builder<0, Is...> {

template <typename T> struct _id {};


inline bool _get(_id<bool>, lua_State *l, const int index) {
return lua_toboolean(l, index);
}
Expand Down Expand Up @@ -155,5 +156,55 @@ typename _pop_n_impl<sizeof...(T), T...>::type _pop_n(lua_State *l) {
return _pop_n_impl<sizeof...(T), T...>::apply(l);
}

template <std::size_t S, typename... Ts>
struct _pop_n_reset_impl {
using type = std::tuple<Ts...>;

template <std::size_t... N>
static type worker(lua_State *l,
_indices<N...>) {
return std::make_tuple(_get(_id<Ts>{}, l, N + 1)...);
}

static type apply(lua_State *l) {
auto ret = worker(l, typename _indices_builder<S>::type());
lua_settop(l, 0);
return ret;
}
};

// Popping nothing returns void
template <typename... Ts>
struct _pop_n_reset_impl<0, Ts...> {
using type = void;
static type apply(lua_State *l) {
lua_settop(l, 0);
}
};

// Popping one element returns an unboxed value
template <typename T>
struct _pop_n_reset_impl<1, T> {
using type = T;
static type apply(lua_State *l) {
T ret = _get(_id<T>{}, l, -1);
lua_settop(l, 0);
return ret;
}
};

template <typename... T>
typename _pop_n_reset_impl<sizeof...(T), T...>::type
_pop_n_reset(lua_State *l) {
return _pop_n_reset_impl<sizeof...(T), T...>::apply(l);
}

template <typename T>
T _pop(_id<T> t, lua_State *l) {
T ret = _get(t, l, -1);
lua_pop(l, 1);
return ret;
}

}
}
23 changes: 18 additions & 5 deletions src/Selector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ void Selector::_check_create_table() {
lua_newtable(_state._l);
};
_put(put);
} else {
lua_pop(_state._l, 1);
}
}

Expand All @@ -31,6 +33,7 @@ void Selector::operator=(const char *s) const {
detail::_push(_state._l, std::string{s});
};
_put(push);
lua_settop(_state._l, 0);
}

Selector::operator bool() const {
Expand All @@ -40,7 +43,9 @@ Selector::operator bool() const {
(*_functor)(1);
_functor.release();
}
return detail::_get(detail::_id<bool>{}, _state._l, -1);
auto ret = detail::_pop(detail::_id<bool>{}, _state._l);
lua_settop(_state._l, 0);
return ret;
}

Selector::operator int() const {
Expand All @@ -50,7 +55,9 @@ Selector::operator int() const {
(*_functor)(1);
_functor.release();
}
return detail::_get(detail::_id<int>{}, _state._l, -1);
auto ret = detail::_pop(detail::_id<int>{}, _state._l);
lua_settop(_state._l, 0);
return ret;
}

Selector::operator unsigned int() const {
Expand All @@ -60,7 +67,9 @@ Selector::operator unsigned int() const {
(*_functor)(1);
_functor.release();
}
return detail::_get(detail::_id<unsigned int>{}, _state._l, -1);
auto ret = detail::_pop(detail::_id<unsigned int>{}, _state._l);
lua_settop(_state._l, 0);
return ret;
}

Selector::operator lua_Number() const {
Expand All @@ -70,7 +79,9 @@ Selector::operator lua_Number() const {
(*_functor)(1);
_functor.release();
}
return detail::_get(detail::_id<lua_Number>{}, _state._l, -1);
auto ret = detail::_pop(detail::_id<lua_Number>{}, _state._l);
lua_settop(_state._l, 0);
return ret;
}

Selector::operator std::string() const {
Expand All @@ -80,7 +91,9 @@ Selector::operator std::string() const {
(*_functor)(1);
_functor.release();
}
return detail::_get(detail::_id<std::string>{}, _state._l, -1);
auto ret = detail::_pop(detail::_id<std::string>{}, _state._l);
lua_settop(_state._l, 0);
return ret;
}

Selector Selector::operator[](const char *name) {
Expand Down
17 changes: 14 additions & 3 deletions test/Test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#include <map>

// The most ghetto unit testing framework ever!
using Test = bool(*)();
// To add a test, author a function with the Test function signature
// and include it in the Map of tests below.
using Test = bool(*)(sel::State &);
using TestMap = std::map<const char *, Test>;
static TestMap tests = {
{"test_function_no_args", test_function_no_args},
Expand Down Expand Up @@ -48,22 +50,31 @@ void ExecuteAll() {
const int num_tests = tests.size();
int passing = 0;
for (auto it = tests.begin(); it != tests.end(); ++it) {
const bool result = it->second();
sel::State state{true};
const bool result = it->second(state);
if (result)
passing += 1;
else
std::cout << "Test \"" << it->first << "\" failed." << std::endl;
int size = state.Size();
if (size != 0)
std::cout << "Test \"" << it->first
<< "\" leaked " << size << " values" << std::endl;
}
std::cout << passing << " out of "
<< num_tests << " tests finished successfully." << std::endl;
}

bool ExecuteTest(const char *test) {
sel::State state{true};
auto it = tests.find(test);
return it->second();
return it->second(state);
}


int main() {
// Executing all tests will run all test cases and check leftover
// stack size afterwards. It is expected that the stack size
// post-test is 0.
ExecuteAll();
}
9 changes: 3 additions & 6 deletions test/class_tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,22 @@ struct Foo {
}
};

bool test_register_class() {
bool test_register_class(sel::State &state) {
Foo foo_instance(1);
sel::State state;
state["foo_instance"].SetObj(foo_instance, "double_add", &Foo::DoubleAdd);
const int answer = state["foo_instance"]["double_add"](3);
return (answer == 8);
}

bool test_mutate_instance() {
bool test_mutate_instance(sel::State &state) {
Foo foo_instance(1);
sel::State state;
state["foo_instance"].SetObj(foo_instance, "set_x", &Foo::SetX);
state["foo_instance"]["set_x"].Call(4);
return (foo_instance.x == 4);
}

bool test_multiple_methods() {
bool test_multiple_methods(sel::State &state) {
Foo foo_instance(1);
sel::State state;
state["foo_instance"].SetObj(foo_instance,
"double_add", &Foo::DoubleAdd,
"set_x", &Foo::SetX);
Expand Down
Loading

0 comments on commit 1b059b5

Please sign in to comment.