Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov <alnsn%yandex.ru@localhost>]

Marc Balmer wrote:
> Module Name:  src
> Committed By: mbalmer
> Date:         Wed Oct 16 19:44:58 UTC 2013
> 
> Added Files:
>       src/sys/modules/lua: Makefile assert.h ctype.h errno.h infinite.lua
>           inttypes.h limits.h locale.h lua.c luaconf.h luavar.h math.h
>           setjmp.h signal.h stdarg.h stddef.h stdio.h stdlib.h string.h
>           test.lua
> 
> Log Message:
> welcome lua(4), a devide driver that can create and control Lua states inside 
> the kernel
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r0 -r1.1 src/sys/modules/lua/Makefile \
>     src/sys/modules/lua/assert.h src/sys/modules/lua/ctype.h \
>     src/sys/modules/lua/errno.h src/sys/modules/lua/infinite.lua \
>     src/sys/modules/lua/inttypes.h src/sys/modules/lua/limits.h \
>     src/sys/modules/lua/locale.h src/sys/modules/lua/lua.c \
>     src/sys/modules/lua/luaconf.h src/sys/modules/lua/luavar.h \
>     src/sys/modules/lua/math.h src/sys/modules/lua/setjmp.h \
>     src/sys/modules/lua/signal.h src/sys/modules/lua/stdarg.h \
>     src/sys/modules/lua/stddef.h src/sys/modules/lua/stdio.h \
>     src/sys/modules/lua/stdlib.h src/sys/modules/lua/string.h \
>     src/sys/modules/lua/test.lua
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

> Added files:
> 
> Index: src/sys/modules/lua/Makefile
> diff -u /dev/null src/sys/modules/lua/Makefile:1.1
> --- /dev/null Wed Oct 16 19:44:58 2013
> +# Compatability code
> +SRCS+=               strcspn.c \
> +             strncat.c \
> +             strpbrk.c \
> +             strspn.c

strcspn searches for "\n\r" when reporting a chunk name in error
handler. We can assume that no kernel scripts are MSDOS files and use
strchr. strncat in the same function. I'm suprised that it's not in the
kernel already. I don't see strspn being used anywhere.

> Index: src/sys/modules/lua/lua.c

I didn't review this file at all.

> Index: src/sys/modules/lua/luaconf.h
> diff -u /dev/null src/sys/modules/lua/luaconf.h:1.1
> --- /dev/null Wed Oct 16 19:44:58 2013
> +++ src/sys/modules/lua/luaconf.h     Wed Oct 16 19:44:57 2013

This file is interesing and I have several comments.

> +#ifndef LUA_CORE
> +#define LUA_CORE
> +#endif

It's not a good idea to define LUA_CORE in luaconf.h file. All Lua core
.c files define LUA_CORE before including luaconf.h. For all other files
it should be undefined.
If you're trying add a new file to a set of Lua core files, you can
define LUA_CORE the same way as other core files do.

> +@@ LUA_INTEGER is the integral type used by lua_pushinteger/lua_tointeger.
> +** CHANGE that if ptrdiff_t is not adequate on your machine. (On most
> +** machines, ptrdiff_t gives a good choice between int or long.)
> +*/
> +#define LUA_INTEGER  ptrdiff_t

It's better to use the same long long type here.

> +#if defined(LUA_USE_READLINE)
> +#include <stdio.h>
> +#include <readline/readline.h>
> +#include <readline/history.h>
> +#define lua_readline(L,b,p)  ((void)L, ((b)=readline(p)) != NULL)
> +#define lua_saveline(L,idx) \
> +     if (lua_strlen(L,idx) > 0)  /* non-empty line? */ \
> +       add_history(lua_tostring(L, idx));  /* add it to history */
> +#define lua_freeline(L,b)    ((void)L, free(b))
> +#else

You can probably change the above #else to #elif !defined(_KERNEL).
It may help you in getting rid of fputs and fflush stubs.

> +#define lua_readline(L,b,p)  \
> +     ((void)L, fputs(p, stdout), fflush(stdout),  /* show prompt */ \
> +     fgets(b, LUA_MAXINPUT, stdin) != NULL)  /* get line */
> +#define lua_saveline(L,idx)  { (void)L; (void)idx; }
> +#define lua_freeline(L,b)    { (void)L; (void)b; }
> +#endif
> +#if LUAI_BITSINT >= 32
> +#define LUAI_UINT32  unsigned int
> +#define LUAI_INT32   int
> +#define LUAI_MAXINT32        INT_MAX
> +#define LUAI_UMEM    size_t
> +#define LUAI_MEM     ptrdiff_t

Because ptrdiff_t is undefined, it's better to use uintptr_t/intptr_t
for LUAI_UMEM/LUAI_MEM.

> +#define LUAL_BUFFERSIZE              BUFSIZ

It's easier to use ifdef _KERNEL rather than defining BUFSZ in your
stdio.h stub file.

> +#ifdef _KERNEL
You need to

#define LUA_NUMBER_LONGLONG

(see also LUA_USELONGLONG below, I'm not yet quite sure whether it's
related)

> +#define LUA_NUMBER   long long
> +
> +#else
> +#define LUA_NUMBER_DOUBLE
> +#define LUA_NUMBER   double
> +#endif
> +#if defined(LUA_CORE)
> +#ifdef _KERNEL
> +#define luai_nummod(a,b)     ((a)%(b))
> +#define luai_numpow(a,b)     luai_nummul(a,b)

a*b != pow(a,b)

> +#if defined(LUA_USELONGLONG)

You probably want to switch to "ll" in the kernel.

> +#undef LUA_CORE

See above.

> +++ src/sys/modules/lua/stddef.h      Wed Oct 16 19:44:57 2013
> @@ -0,0 +1,16 @@
> +/*   $NetBSD */
> +
> +/*
> + * This file is a placeholder only, to allow Lua to be compiled from
> + * unchanged sources.
> + */
> +
> +#include <sys/types.h>
> +
> +#ifdef  _BSD_PTRDIFF_T_
> +typedef _BSD_PTRDIFF_T_              ptrdiff_t;
> +#undef  _BSD_PTRDIFF_T_
> +#endif

ptrdiff_t isn't defined in the kernel and I bet there is a reason why
it's undefined. Unfortuntely, it's used quite a lot in Lua (with Lua
small codebase it'll probably take a couple of minutes to change all
ptrdiff_t occurences manually and even faster with sed/perl). The best
type I can think of is intptr_t.

> Index: src/sys/modules/lua/stdlib.h
> diff -u /dev/null src/sys/modules/lua/stdlib.h:1.1
> --- /dev/null Wed Oct 16 19:44:58 2013
> +++ src/sys/modules/lua/stdlib.h      Wed Oct 16 19:44:57 2013

> +#define exit(EXIT_FAILURE)   return

You only need to make a change in one place in ldo.c:

@@ -105,7 +110,11 @@ void luaD_throw (lua_State *L, int errco
       lua_unlock(L);
       G(L)->panic(L);
     }
+#if defined(_KERNEL)
+    panic("luaD_throw(), errcode=%d", errcode);
+#else
     exit(EXIT_FAILURE);
+#endif
   }
 }

Alex