You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Akins, Brian" <Br...@turner.com> on 2007/04/27 19:48:23 UTC
[PATCH] mod_wombat: add table_get and table_set
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.
--
Brian Akins
Chief Operations Engineer
Turner Digital Media Technologies
Re: [PATCH] mod_wombat: add table_get and table_set
Posted by "Akins, Brian" <Br...@turner.com>.
On 4/27/07 2:34 PM, "Brian McCallister" <br...@apache.org> 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
Re: [PATCH] mod_wombat: add table_get and table_set
Posted by "Akins, Brian" <Br...@turner.com>.
On 5/4/07 7:42 PM, "Rici Lake" <ri...@ricilake.net> 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
Posted by Rici Lake <ri...@ricilake.net>.
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
Posted by "Akins, Brian" <Br...@turner.com>.
On 4/30/07 5:53 PM, "Brian McCallister" <br...@apache.org> 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
Posted by Brian McCallister <br...@apache.org>.
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
Posted by "Akins, Brian" <Br...@turner.com>.
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
Re: [PATCH] mod_wombat: add table_get and table_set
Posted by "Akins, Brian" <Br...@turner.com>.
On 4/27/07 2:34 PM, "Brian McCallister" <br...@apache.org> 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
Re: [PATCH] mod_wombat: add table_get and table_set
Posted by Brian McCallister <br...@apache.org>.
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