Page MenuHomeFreeBSD

flua: add freebsd module implementing kldload/kldunload
ClosedPublic

Authored by bapt on Sep 6 2024, 9:05 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 4:50 AM
Unknown Object (File)
Wed, Nov 6, 7:55 AM
Unknown Object (File)
Sun, Oct 20, 9:19 AM
Unknown Object (File)
Sun, Oct 20, 9:19 AM
Unknown Object (File)
Sun, Oct 20, 9:18 AM
Unknown Object (File)
Sun, Oct 20, 9:18 AM
Unknown Object (File)
Sun, Oct 20, 9:18 AM
Unknown Object (File)
Sun, Oct 20, 8:57 AM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bapt requested review of this revision.Sep 6 2024, 9:05 AM

Under my proposal, these should live under freebsd.sys.linker or freebsd.c.sys.linker. I know it feels like overkill, but consider that linker.h has 8 different system calls relating to KLDs, plus some flags, and we would eventually add those as well. IMO it's not right to cram everything into the top-level namespace.

lib/flua/libfreebsd/lua_freebsd.c
53 ↗(On Diff #143013)

The convention from lua posix is to return true upon success, or nil, strerror(errno), errno upon failure. I think we should follow that for system call wrappers as well.

65 ↗(On Diff #143013)

Why bother with kldfind() first?

Under my proposal, these should live under freebsd.sys.linker or freebsd.c.sys.linker. I know it feels like overkill, but consider that linker.h has 8 different system calls relating to KLDs, plus some flags, and we would eventually add those as well. IMO it's not right to cram everything into the top-level namespace.

I'm not seeing the value of freebsd.c.sys.linker. I was going to reply to the thread that I had a hard time thinking of any time we'd need freebsd.c.sys since freebsd.sys would always suffice (eg, we don't have any times we'd want to expose where we'd have freebsd.sys that's different than freebsd.c.sys). Also, I'm not entirely sure we'd even need .sys since in a lua program I can't imagine needing to know if it's a system call or a library call and having collisions between the two in the freebsd namespace, and if we did, then the non-libc version should get a freebsd.util or freebsd.mumble disambiguation. But I could go either way on that detail (eg freebsd.sys just means 'libc and/or libsys' since libsys is a impl detail of libc that nobody is going to care about. We can't have a lua that's linked to libsys, but not libc given what lua picks up from libc today.

Anyway, I'm find with freebsd.<syscall> even, were it come to that, with freebsd.<big-class>.syscall for things like jails, capsicim, etc. But having said that, I'll let the people pushing this decide the finer points.

lib/flua/libfreebsd/lua_freebsd.c
53 ↗(On Diff #143013)

Perhaps we should have a wrapper in flua / boot loader for that? (I say boot loader because some of these routines might wind up being implemented there, though not many, we'd want the same conventions... I'm happy to take care of the boot loader side of things if we do have this in flua and it becomes an issue). Of course, not relevant for this commit (though maybe we want to have a wrapper around the loader's load stuff, but I'm having trouble seeing a huge need for that).

tl;dr: Let's have a wrapper to do the value or (nil, string, errno) and value or (nil, strerror(errno), errno) thing.

Modulo namespace issues from mark, I'm good with this as is... Also question the need for kldfind, but I don't know if it catches some edge cases or not.

This revision is now accepted and ready to land.Sep 6 2024, 2:22 PM

move to freebsd.sys.linker

make kldunload accept both fileid and name

This revision now requires review to proceed.Sep 6 2024, 2:37 PM
bapt marked 2 inline comments as done.Sep 6 2024, 2:43 PM
bapt added inline comments.
lib/flua/libfreebsd/lua_freebsd.c
65 ↗(On Diff #143013)

for userfriendlyness but fixed supporting both fileid and name

bapt marked an inline comment as done.

Add manpage

This revision was not accepted when it landed; it landed in state Needs Review.Sep 6 2024, 3:29 PM
This revision was automatically updated to reflect the committed changes.