Re: [PATCH 1/1] Bind sigaction(2)
On Sun, Apr 21, 2019 at 19:02:07 +0100, Richard Ipsum wrote:
diff --git a/luxio.c b/luxio.c
+
+static struct {
+ lua_State *state;
+ lua_Hook orighook;
+ int orighookmask;
+ int orighookcount;
+ sigset_t origset;
+ int signo;
+ int handlers[512];
Is 512 a defined limit on handlers, or is this a magic number you've derived
from somewhere else? If the latter, can it be a define (LUXIO_MAX_SIGNAL) or
somesuch?
+} luxio__signal_ctx;
Currently this approach means that only one Lua VM in a given process could
have signal handlers registered -- I'm "okay" with that, but it's worth
documenting the limitation.
+static void luxio__sigaction_handler(int signo)
Is there value in being an `sa_sigaction` handler and saving / passing-to-lua
any of the siginfo_t ? If not, no worries.
+static int
+luxio_sigaction(lua_State *L)
Documentation comment for this please, so that the generated docs can
explain both what a signal is, what the limitations are on it, etc.
+{
+ int signo;
+ struct sigaction sa = {
+ .sa_handler = luxio__sigaction_handler
+ };
+
+ signo = luaL_checkinteger(L, 1);
This number can end up out of the range of valid signals (e.g. less than one
or more than your 512 above).
+
+ if (!lua_isfunction(L, 2)) {
+ lua_pushinteger(L, -1);
+ lua_pushinteger(L, EINVAL);
+ return 2;
+ }
Would it make sense to support nil in this position to mean "deregister handler"
?
---
This looks like a reasonable approach over-all and I'd support merging it with
the above concerns resolved (such as deregistering handlers, dealing with the
limits, etc).
Thank you, and again I apologise for taking so long to respond.
D.
--
Daniel Silverstone http://www.digital-scurf.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69