Date: Mon, 2 Jun 2014 22:26:12 -0300 From: Pedro Arthur <bygrandao@gmail.com> To: "Wojciech A. Koszek" <wkoszek@freebsd.org> Cc: svn-soc-all@freebsd.org, pedrosouza@freebsd.org Subject: Re: socsvn commit: r268635 - soc2014/pedrosouza/lua_loader/head/sys/boot/common Message-ID: <CAKN1MR6nr9hF_oPcpbNLSz-gEvg3rmZzm1gNOPxvXLgW0XOS1g@mail.gmail.com> In-Reply-To: <20140602220344.GB2587@FreeBSD.org> References: <201405261337.s4QDbYL7079776@socsvn.freebsd.org> <20140602220344.GB2587@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
I'll do these fixes. The new functions do the following: lua_realloc -> this function is passed to lua_newstate to realloc mem, it changed from the previous lua version where it wasn't needed. data_chunk struct and read_chunk -> are needed to execute a string, as the lua_load function expects a function that reads a stream and return the size read, data_chunk contains a pointer to the string and its size. When I call lua_load(read_chunk, data_chunk) lua_load calls read_chunk(data_chunk, &out_size) and read_chunk just returns the pointer to the string and its size. 2014-06-02 19:03 GMT-03:00 Wojciech A. Koszek <wkoszek@freebsd.org>: > On Mon, May 26, 2014 at 01:37:34PM +0000, pedrosouza@freebsd.org wrote: > > Author: pedrosouza > > Date: Mon May 26 13:37:34 2014 > > New Revision: 268635 > > URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=268635 > > > > Log: > > Added missing file intep_lua.c > > > Pedro, > > My comments below. > > > > > Added: > > soc2014/pedrosouza/lua_loader/head/sys/boot/common/interp_lua.c > > > > Added: soc2014/pedrosouza/lua_loader/head/sys/boot/common/interp_lua.c > > > ============================================================================== > > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > > +++ soc2014/pedrosouza/lua_loader/head/sys/boot/common/interp_lua.c > Mon May 26 13:37:34 2014 (r268635) > > @@ -0,0 +1,203 @@ > > +#include <sys/cdefs.h> > > Please put our copyright in here. > > > +__FBSDID("$FreeBSD$"); > > + > > +#include <sys/param.h> /* to pick up __FreeBSD_version */ > > +#include <machine/stdarg.h> > > + > > + > > +#include "bootstrap.h" > > +#include "interp.h" > > + > > +#define lua_c > > + > > +#include "../lua/src/lua.h" > > +#include "../lua/src/ldebug.h" > > + > > +struct interp_lua_softc { > > + lua_State *luap; > > +}; > > +struct interp_lua_softc lua_softc = { 0 }; > > + > > +#define LDBG(...) do { \ > > + printf("%s(%d): ", __func__, __LINE__); \ > > + printf(__VA_ARGS__); \ > > + printf("\n"); \ > > +} while (0) > > + > > + > > +int lua_print(lua_State *L) > > +{ > > + int i; > > + int n = lua_gettop(L); > > + for (i = 1; i <= n; ++i) > > + printf("%s", lua_tostring(L, i)); > > + return 0; > > +} > > This comment touches the whole file: something is wrong with formatting. > We'd appreciate if you could look at it a bit more and maybe align the > formatting accross the file. > > > + > > +int lua_perform(lua_State *L) > > +{ > > + > > + int argc, ret; > > + char **argv; > > ..like here.. > > > + int res = -1; > > + int n = lua_gettop(L); > > + > > + if (n >= 1) > > + { > > ..or here.. > > > + parse(&argc, &argv, lua_tostring(L, 1)); > > + res = perform(argc, argv); > > + } > > + lua_pushnumber(L, res); > > + > > + return 1; > > +} > > + > > +void * lua_realloc(void *ud, void *ptr, size_t osize, size_t nsize) > > +{ > > + (void)ud; (void)osize; /* not used */ > > + if (nsize == 0) { > > + free(ptr); > > + return NULL; > > + } > > + else > > + return realloc(ptr, nsize); > > +} > > + > > +typedef struct data_chunk > > +{ > > + void * data; > > + size_t size; > > +} data_chunk; > > + > > +const char * read_chunk(lua_State *L, void * chunk, size_t *sz) > > +{ > > + data_chunk * ds = (data_chunk *)chunk; > > + if (ds->size == 0) return NULL; > > + *sz = ds->size; > > + ds->size = 0; > > + return (const char*)ds->data; > > +} > > > I don't seem to recall my code had these functions. Can you quickly > summarize what they do? > > > + > > + > > +int do_string(lua_State *L, const char * str, size_t size) > > +{ > > + int res; > > + data_chunk ds; > > + ds.data = (void*)str; > > + ds.size = size; > > + res = lua_load(L, read_chunk, &ds, "do_string___", 0); > > + res = lua_pcall(L, 0, LUA_MULTRET, 0); > > + return res; > > +} > > + > > + > > +void > > +interp_lua_init(void *ctx) > > +{ > > + lua_State *luap; > > + struct bootblk_command **cmdp; > > + struct interp_lua_softc *softc; > > + struct env_var *ev; > > + const char *name_str, *val_str; > > + char buf[16]; > > + > > + softc = ctx; > > + luap = lua_newstate(lua_realloc, NULL); > > + if (luap == NULL) { > > + LDBG("problem with initializing Lua interpreter\n"); > > + } > > + softc->luap = luap; > > + lua_register(luap, "print", lua_print); > > + lua_register(luap, "perform", lua_perform); > > + > > +} > > + > > +int > > +interp_lua_run(void *data, const char *line) > > +{ > > + lua_State *luap; > > + struct interp_lua_softc *softc; > > + int argc, ret; > > + char **argv; > > + > > + softc = data; > > + luap = softc->luap; > > + > > + if (do_string(luap, line, strlen(line)) != 0) > > + printf("[LUA]Failed to execure \'%s\'\n", line); > > + > > + return (0); > > +} > > + > > +int > > +interp_lua_incl(void *ctx, const char *filename) > > +{ > > + lua_State *luap; > > + struct interp_lua_softc *softc; > > + struct stat st; > > + char *filebuf, *errstr; > > + int fd, filebufoff, i, rleft, rread; > > + > > + printf("[Lua] Including file %s.\n", filename); > > + /* > > If you could look into using #ifdef for commenting out, this would be good. > > > + if ((strcmp(filename, "/boot/loader.rc") == 0) || > > + (strcmp(filename, "/boot/boot.conf") == 0)) { > > + printf("Skipping loader.rc and boot.conf"); > > + return (0); > > + } > > There was a small glitch here: I think the current implementation is that > if > there's no Forth interpreter, the rc/conf (which are in Forth) will get > passed to the loader as well. > > I'd keep it that way in case somebody takes advantage of this bug, but we > could comment this part out. > > > + */ > > + softc = ctx; > > + luap = softc->luap; > > + /* > > + > > + fd = open(filename, O_RDONLY); > > + if (fd < 0) { > > + printf("Couldn't open file %s\n", filename); > > + return (1); > > + } > > + i = stat(filename, &st); > > + assert(i == 0); > > + filebuf = malloc(st.st_size); > > + assert(filebuf != NULL); > > + memset(filebuf, 0, st.st_size); > > + */ > > + /* > > + * XX: Investigate stat() vs logic problem. I'm getting > > + * more bytes that the file really has. > > + */ > > + > > + /* > > + rleft = st.st_size - 1; > > + filebufoff = 0; > > + for (;;) { > > + rread = read(fd, filebuf + filebufoff, rleft); > > + assert(rread >= 0); > > + rleft -= rread; > > + filebufoff += rread; > > + if (rread == 0 || rleft <= 0) { > > + break; > > + } > > + } > > + close(fd); > > + i = LUA_DOSTRING(luap, filebuf); > > + free(filebuf); > > + if ((i != 0) && (lua_isnil(luap, -1) == 0)) { > > + errstr = lua_tostring(luap, -1); > > + if (errstr == NULL) { > > + errstr = "internal error; errstr must be string"; > > + } > > + printf("Problem with script execution:\n\n"); > > + printf("\t'%s'\n\n", errstr); > > + lua_pop(luap, 1); > > + } > > + */ > > + return (0); > > +} > > + > > + > > +struct interp boot_interp_lua = { > > + .init = interp_lua_init, > > + .run = interp_lua_run, > > + .incl = interp_lua_incl, > > + .context = &lua_softc, > > +}; > > Other than that it looks good. > > > -- > Wojciech A. Koszek > wkoszek@FreeBSD.czest.pl > http://FreeBSD.czest.pl/~wkoszek/ >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAKN1MR6nr9hF_oPcpbNLSz-gEvg3rmZzm1gNOPxvXLgW0XOS1g>