Date: Mon, 2 Jun 2014 22:03:45 +0000 From: "Wojciech A. Koszek" <wkoszek@freebsd.org> To: pedrosouza@freebsd.org Cc: svn-soc-all@freebsd.org Subject: Re: socsvn commit: r268635 - soc2014/pedrosouza/lua_loader/head/sys/boot/common Message-ID: <20140602220344.GB2587@FreeBSD.org> In-Reply-To: <201405261337.s4QDbYL7079776@socsvn.freebsd.org> References: <201405261337.s4QDbYL7079776@socsvn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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?20140602220344.GB2587>