Skip site navigation (1)Skip section navigation (2)
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>