provide automatic nop fallback for missing callbacks #338

Zapper9982 · 2025-12-21T17:42:14Z

Changes:

Added:

lunatik_nop1 : A generic no-op lua_CFunction in lunatik.h`
lunatik_defaultfield : reusable helper to default table fields to a function
Modified device.new to automatically apply lunatik_nop when callbacks are missing

Testing :

Verified with devices using default callbacks
Verified custom callbacks are preserved
Verified mixed scenarios (custom + default)
All tests pass with kernel module loaded successfully

Screenshots :

lneto · 2025-12-21T17:59:06Z

lib/luadevice.c

+	lunatik_defaultfield(L, 1, "open", lunatik_nop);
+	lunatik_defaultfield(L, 1, "release", lunatik_nop);


this wasn't what I meant.. I meant to just failover on the call.. not to check the field and store it.. look for the place we are actually calling and failing when such callbacks aren't defined and instead of failing (if type(field) is not function, use luantik_nop.

okay yeah makes sense , i misinterpreted the requirement , but got it now

lneto

Please, squash your commits into a single one. Could you also review Luadevice documentation to see if it needs to be updated? Thanks!

lneto · 2025-12-21T23:06:19Z

examples/systrack.lua


 local syscalls = {"openat", "read", "write", "readv", "writev", "close"}

 local function nop() end -- do nothing


Unused, right?

okay , so the runtime fallback is added for device.new and not probe.new so ideally if i remove that, it will break .
so i am wondering if i should add a similar handler for probe as well ?

ah.. it's used on probe.new below, right? yeah, if it's apply, I think we should do this change as well.. and perhaps add a new macro that could be used for both.. take a look at optinteger().

Zapper9982 · 2025-12-22T03:59:55Z

Please, squash your commits into a single one. Could you also review Luadevice documentation to see if it needs to be updated? Thanks!

Yes sure will do that

lneto · 2025-12-22T22:16:57Z

lunatik.h

+
+

please, mind our style..

Suggested change

lneto · 2025-12-22T22:23:44Z

lunatik.h

+#define lunatik_optfunction(L, idx, field, default_func) \
+	((lua_getfield((L), (idx), (field)) == LUA_TFUNCTION) ? \
+		(void)0 : \
+		(lua_pop((L), 1), lua_pushcfunction((L), (default_func))))


any reason for not following the do { } while (0) approach? btw, I would create a template, e.g., NEWOPT and create optcfunction and optinteger using it.

any reason for not following the do { } while (0) approach? btw, I would create a template, e.g., NEWOPT and create optcfunction and optinteger using it.

I thought the ternary operator would do the trick. At the end, both achieve the same result functionally, but you're right that do-while-0 is more idiomatic for statement-like macros.

I'll go ahead and make a NEWOPT template .

@lneto but , isnt optinteger being used to access struct fields , while in optcfunction the ideal requirement , is accessing Lua Table fields , so i think its better just refactor optcfunction .

@Zapper9982 you are right. Perhaps, we should change optinteger instead to just get the field and have another macro calling it to set the struct field, that's breaking it into two macros. What do you think? (of course, we can tackle it as a separate PR).

Breaking lunatik_optinteger into a getter macro (that just pushes the value or the default onto the stack) and an assignment wrapper would make it much more composable for sure

lneto · 2025-12-22T22:25:22Z

lunatik.h

+	return 0;
+}
+
+#define lunatik_optfunction(L, idx, field, default_func) \


please move it next to optinteger, it will make easier to find this API in the header later =)

lneto · 2025-12-23T11:45:26Z

lunatik.h

+#define lunatik_optcfunction(L, idx, field, default_func)		\
+do {									\
+	(lua_getfield(L, idx, field) == LUA_TFUNCTION) ?		\
+		(void)0 :						\
+		(lua_pop(L, 1), lua_pushcfunction(L, default_func));	\
+} while (0)


Suggested change

#define lunatik_optcfunction(L, idx, field, default_func) \

do { \

(lua_getfield(L, idx, field) == LUA_TFUNCTION) ? \

(void)0 : \

(lua_pop(L, 1), lua_pushcfunction(L, default_func)); \

} while (0)

static inline void lunatik_optcfunction(L, idx, field, opt)

{

if (lua_getfield(L, idx, field) != LUA_TFUNCTION) {

lua_pop(L, 1);

lua_pushcfunction(L, opt));

}

}

I'm not fond of using the same name pattern as optinteger as we are not actually setting a struct field as pointed out, but I couldn't think on something better.. anyway, in this case, we don't need a macro.. an inline will do =). Please, move it to next optinteger anyway, it will be easier to maintain. Thanks.

lneto

It's looking good to me.. the only two things are missing is 1) to move the function declaration to the region I mentioned and 2) squash your commits. Thanks!

Zapper9982 · 2025-12-23T15:58:47Z

It's looking good to me.. the only two things are missing is 1) to move the function declaration to the region I mentioned and 2) squash your commits. Thanks!

Its already there :))

but yes ill squash my commits now , was waiting on getting the final green flag , so ill squash them at once

lneto · 2025-12-23T21:59:10Z

lib/luadevice.c

-		pr_err("%s: operation isn't defined for /dev/%s\n", fop, lua_tostring(L, -1));
-		goto err;
-	}
+


Suggested change

plase mind that, we don't do double \n

lneto

sorry.. I was about to merge and found that \n, can you fix it? then I can merge.. also, could you remove this "feat(core)" from your commit message? we don't use this kind of prefix.. please, check our history to get a better sense of our commit messages..

lneto reviewed Dec 21, 2025

View reviewed changes

lneto requested changes Dec 21, 2025

View reviewed changes

lneto reviewed Dec 21, 2025

View reviewed changes

Zapper9982 force-pushed the nop-impl branch from 8eb117a to e593e40 Compare December 22, 2025 19:48

Zapper9982 requested a review from lneto December 22, 2025 19:57

lneto reviewed Dec 22, 2025

View reviewed changes

lunatik.h Outdated

Comment on lines 114 to 115

Copy link

Contributor

lneto Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, mind our style..

Suggested change

lneto reviewed Dec 22, 2025

View reviewed changes

lneto reviewed Dec 23, 2025

View reviewed changes

Zapper9982 requested a review from lneto December 23, 2025 15:32

lneto reviewed Dec 23, 2025

View reviewed changes

Zapper9982 force-pushed the nop-impl branch from 49a24c3 to 242fc51 Compare December 23, 2025 17:03

Zapper9982 requested a review from lneto December 23, 2025 17:04

lneto approved these changes Dec 23, 2025

View reviewed changes

lneto reviewed Dec 23, 2025

View reviewed changes

lneto requested changes Dec 23, 2025

View reviewed changes

provide automatic nop fallback for missing callbacks

116845f

Zapper9982 force-pushed the nop-impl branch from 242fc51 to 116845f Compare December 24, 2025 14:44

Zapper9982 changed the title ~~feat(device) : add nop default for open/release callbacks~~ provide automatic nop fallback for missing callbacks Dec 24, 2025

lneto approved these changes Dec 24, 2025

View reviewed changes

lneto merged commit 94bd2b6 into luainkernel:master Dec 24, 2025
1 check passed

		lunatik_defaultfield(L, 1, "open", lunatik_nop);
		lunatik_defaultfield(L, 1, "release", lunatik_nop);


		local syscalls = {"openat", "read", "write", "readv", "writev", "close"}

		local function nop() end -- do nothing

provide automatic nop fallback for missing callbacks #338

provide automatic nop fallback for missing callbacks #338

Conversation

Zapper9982 commented Dec 21, 2025

Changes:

Testing :

Screenshots :

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lneto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zapper9982 commented Dec 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lneto left a comment

Choose a reason for hiding this comment

Uh oh!

Zapper9982 commented Dec 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lneto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants