Re: [PATCH] mod_wombat: add table_get and table_set
On 5/4/07 7:42 PM, Rici Lake [EMAIL PROTECTED] wrote:
In Lua, setting to nil is equivalent to deletion. So I think this should be:
if (lua_isnoneornil(L, 3)
apr_table_unset(t, key);
else {
const char *val = luaL_checkstring(L, 3);
apr_table_set(t, key, val);
}
+1
if (val == NULL)
lua_pushnil(L);
else
lua_pushstring(L, val);
+1
Agreed. Also, it's misleading -- they are APR tables, not Lua tables.
Brian M. changed this before committing, I think.
This is poor Lua style. You shouldn't assume (or require) the stack to
be empty at the start of the function. Use top-relative (negative) indices
instead -- they are no slower.
Until a day before I submitted patch, I had never touched Lua, so I'm sure I
have bad style and form :)
Also use lua_{get,set}field for clarity, and luaL_register (luaL_openlib
is deprecated).
I had been using the PiL 5.0 for docs and lua-users.org was down most of
last week. I got hard copy of 5.1 PiL this weekend, so maybe my style will
improve ;)
Thanks for all the pointers. I think mod_wombat is almost in a state where
I can start actually working on the original problem I needed to solve.
Question: what would be the best way to load other modules to mod_wombat
(not apache modules). For example, I want/need to load the lua pcre stuff,
but I don't want to write a new apache module just to hook that into
mod_wombat. I am new to Lua, so I am sure there is a better way. Can ping
me off list if this seems OT for httpd-dev
--
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies
Re: [PATCH] mod_wombat: add table_get and table_set
A few comments intermingled into the patch:
Brian McCallister wrote:
On Apr 30, 2007, at 2:02 PM, Akins, Brian wrote:
--- apr_lua.c (revision 0)
+++ apr_lua.c (revision 0)
@@ -0,0 +1,55 @@
+#include apr.h
+#include apr_tables.h
+
+#include lua.h
+#include lauxlib.h
+#include lualib.h
+
+#define lua_unboxpointer(L,i) (*(void **)(lua_touserdata(L, i)))
+
+static apr_table_t* check_apr_table(lua_State* L, int index) {
+luaL_checkudata(L, index, Apr.Table);
+apr_table_t* t = (apr_table_t*)lua_unboxpointer(L, index);
+return t;
+}
+
+static int lua_table_set(lua_State* L) {
+apr_table_t *t = check_apr_table(L, 1);
+const char* key = luaL_checkstring(L, 2);
+const char* val = luaL_checkstring(L, 3);
+
+apr_table_set(t, key, val);
In Lua, setting to nil is equivalent to deletion. So I think this should be:
if (lua_isnoneornil(L, 3)
apr_table_unset(t, key);
else {
const char *val = luaL_checkstring(L, 3);
apr_table_set(t, key, val);
}
+return 0;
+}
+
+static int lua_table_get(lua_State* L) {
+apr_table_t *t = check_apr_table(L, 1);
+const char* key = luaL_checkstring(L, 2);
+const char *val = apr_table_get(t, key);
val might be NULL; the function should return nil in that case:
if (val == NULL)
lua_pushnil(L);
else
lua_pushstring(L, val);
+lua_pushstring(L, val);
+return 1;
+}
+
+static const luaL_reg lua_table_methods[] = {
+{set, lua_table_set},
+{get, lua_table_get},
+{0, 0}
+};
Even though these are static, we might want to be careful in naming
as these are reaching into lua's namespace (lua_* and luaL_*).
Agreed. Also, it's misleading -- they are APR tables, not Lua tables.
+
+
+int apr_lua_init(lua_State *L, apr_pool_t *p) {
+luaL_newmetatable(L, Apr.Table);
+luaL_openlib(L, apr_table, lua_table_methods, 0);
+lua_pushstring(L, __index);
+lua_pushstring(L, get);
+lua_gettable(L, 2);
+lua_settable(L, 1);
+
+lua_pushstring(L, __newindex);
+lua_pushstring(L, set);
+lua_gettable(L, 2);
+lua_settable(L, 1);
+
+return 0;
+}
This is poor Lua style. You shouldn't assume (or require) the stack to
be empty at the start of the function. Use top-relative (negative) indices
instead -- they are no slower.
Also use lua_{get,set}field for clarity, and luaL_register (luaL_openlib
is deprecated).
luaL_register(L, apr_table, lua_table_methods);
luaL_newmetatable(L, Apr.Table);
lua_getfield(L, -2, get);
lua_setfield(L, -2, __index);
lua_getfield(L, -2, set);
lua_setfield(L, -2, __newindex);
Re: [PATCH] mod_wombat: add table_get and table_set
On 4/30/07 5:53 PM, Brian McCallister [EMAIL PROTECTED] wrote: I would like to maintain a function which is analogous to lua_pushstring() and lua_pushinteger() for pushing the request_rec into a function call or whatnot from the C side. Will this work with the hook? (I am a hook newb). Sure. The way I have it now, it calls the push function first in the hook. We could move that outside the hook and still have the hook available. Even though these are static, we might want to be careful in naming as these are reaching into lua's namespace (lua_* and luaL_*). Sure, we can rename these to apr_lua_table_methods and prefix the methods with apr_. Why pass the pool in (other than matching the hook form, but this isn't invoked via ) I added thepool because I'm sure at some point I will need it and didn't want to rewrite anything that already called it. I know not a good reason... -- Brian Akins Chief Operations Engineer Turner Digital Media Technologies
Re: [PATCH] mod_wombat: add table_get and table_set
On 4/27/07 2:34 PM, Brian McCallister [EMAIL PROTECTED] wrote: We may want to consider not putting table_set and table_get on the request, though. It might be better to have a general purpose userdata type (metatable) for apr_table_t and put the functions there. This would allow for something like: function handle(r) r.headers_out['Lua'] = 'Cool' val = r.headers_in['User-Agent'] end Here's the patch that does just that. Ugly, I'm sure. I know lua now, and I know C. Still having issues stitching them together... -- Brian Akins Chief Operations Engineer Turner Digital Media Technologies request-table.diff Description: Binary data
Re: [PATCH] mod_wombat: add table_get and table_set
Probably more changes than needs to be in one patch: - use hooks for: -- wombat_open - called by create_vm -- wombat_request - called instead of apw_request_push -added apr_lua.c and .h - only handles tables for now. Can be extended to do more in future. -- Brian Akins Chief Operations Engineer Turner Digital Media Technologies wombat_hooks.diff Description: Binary data
Re: [PATCH] mod_wombat: add table_get and table_set
On Apr 30, 2007, at 2:02 PM, Akins, Brian wrote:
Probably more changes than needs to be in one patch:
- use hooks for:
-- wombat_open - called by create_vm
+1 Perfect!
-- wombat_request - called instead of apw_request_push
I would like to maintain a function which is analogous to
lua_pushstring() and lua_pushinteger() for pushing the request_rec
into a function call or whatnot from the C side.
Will this work with the hook? (I am a hook newb).
-added apr_lua.c and .h - only handles tables for now. Can be
extended to do
more in future.
Index: apr_lua.c
===
--- apr_lua.c (revision 0)
+++ apr_lua.c (revision 0)
@@ -0,0 +1,55 @@
+#include apr.h
+#include apr_tables.h
+
+#include lua.h
+#include lauxlib.h
+#include lualib.h
+
+#define lua_unboxpointer(L,i) (*(void **)(lua_touserdata(L, i)))
+
+static apr_table_t* check_apr_table(lua_State* L, int index) {
+luaL_checkudata(L, index, Apr.Table);
+apr_table_t* t = (apr_table_t*)lua_unboxpointer(L, index);
+return t;
+}
+
+static int lua_table_set(lua_State* L) {
+apr_table_t *t = check_apr_table(L, 1);
+const char* key = luaL_checkstring(L, 2);
+const char* val = luaL_checkstring(L, 3);
+
+apr_table_set(t, key, val);
+return 0;
+}
+
+static int lua_table_get(lua_State* L) {
+apr_table_t *t = check_apr_table(L, 1);
+const char* key = luaL_checkstring(L, 2);
+const char *val = apr_table_get(t, key);
+lua_pushstring(L, val);
+return 1;
+}
+
+static const luaL_reg lua_table_methods[] = {
+{set, lua_table_set},
+{get, lua_table_get},
+{0, 0}
+};
Even though these are static, we might want to be careful in naming
as these are reaching into lua's namespace (lua_* and luaL_*).
+
+
+int apr_lua_init(lua_State *L, apr_pool_t *p) {
+luaL_newmetatable(L, Apr.Table);
+luaL_openlib(L, apr_table, lua_table_methods, 0);
+lua_pushstring(L, __index);
+lua_pushstring(L, get);
+lua_gettable(L, 2);
+lua_settable(L, 1);
+
+lua_pushstring(L, __newindex);
+lua_pushstring(L, set);
+lua_gettable(L, 2);
+lua_settable(L, 1);
+
+return 0;
+}
Why pass the pool in (other than matching the hook form, but this
isn't invoked via ) and what is the general policy on borrowing from
the apr namespace for an exported function?
-Brian
Re: [PATCH] mod_wombat: add table_get and table_set
On Apr 27, 2007, at 10:48 AM, Akins, Brian wrote: Probably not the best way to do this, but adds ability to get/set on r-headers_in and r-headers_out Example usage: function handle(r) r:table_set(r.headers_out, Lua, Cool); val = r:table_get(r.headers_in, User-Agent); r:puts(User-Agent: .. val .. \n); End FWW, I had never done any lua until this morning, so I'm sure it can be done better. I'm not a huge fan of just wrapping apr_table_get/set, but wasn't sure if I shoved them into a lua table that I could keep them (the apr table and the lua table) in sync. Very nice, thank you! We may want to consider not putting table_set and table_get on the request, though. It might be better to have a general purpose userdata type (metatable) for apr_table_t and put the functions there. This would allow for something like: function handle(r) r.headers_out['Lua'] = 'Cool' val = r.headers_in['User-Agent'] end where we use an apw_push_apr_table(L, r-headers_out) to push the headers onto the Lua request and apw_check_apr_table(...) to pull it back off. This would save special cases for all the additional apr_table_t usages, as well. Thoughts? -Brian
Re: [PATCH] mod_wombat: add table_get and table_set
On 4/27/07 2:34 PM, Brian McCallister [EMAIL PROTECTED] wrote:
Thoughts?
Sounds good to me. Like I said, I just started playing with it this morning
:)
If you can point me more in the right direction, I can give it a try. I was
just scratching an itch.
Also, I want to add quick_handler to the mix. If I'm reading it correctly,
wombat_handler only uses the stat caching, so I was using the harness stuff
basically like this:
int wombat_quick_harness(request_rec *r, int lookup) {
if(lookup) {
return DECLINED;
}
return wombat_request_rec_hook_harness(r, quick);
}
static const char* register_quick_hook(cmd_parms *cmd, void *_cfg, const
char *file, const char *function) {
return register_named_file_function_hook(quick, cmd, _cfg, file,
function);
}
I want to be able to cache forever
Thoughts about that? Doesn't seem ideal the way I was doing it...
--
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies
