flua: clean up lposix argument checking

[ifreund_freebsdfoundation.org]

Summary

The key insight here is that the luaL_check*() and luaL_opt*() functions
will happily take indexes that are larger than the stack top and print a
useful error message.

This means that there is no need to check if too few arguments have been
received prior to checking the types of individual arguments.

This patch also replaces a couple reimplementations of luaL_opt*()
functions with the luaL helpers.

References: https://www.lua.org/manual/5.4/manual.html#4.1.2
Sponsored by: The FreeBSD Foundation

[emaste]

This looks reasonable to me but I'd defer to @kevans

libexec/flua/modules/lposix.c

42–43

So luaL_checkinteger will fail (and not return) for 0 args, and the enforce_max_args will fail for > 1 args?

[ifreund_freebsdfoundation.org]

libexec/flua/modules/lposix.c

42–43

Yep, exactly. For the first case the error from luaL_checkinteger() looks like this:

> unistd.read()
stdin:1: bad argument #1 to 'read' (number expected, got no value)

For the latter case the error from enforce_max_args() looks like this:

> unistd.read(0, 0, 0)
stdin:1: bad argument #3 to 'read' (too many arguments)

[emaste]

libexec/flua/modules/lposix.c

154–155

I don't think it matters much but it would seem better to do all of the arg format/presence checks first, before other validity checks. At least in cases where it's straightforward to do so, so perhaps not chown above.

That is, for pclose it seems reasonable to me for "pclose(-1, 1)` to fail from the extra arg rather than the invalid fd.

Or - perhaps we can just check for too many args first in each fn?

[emaste]

I'm fine with this as is, but will wait a little while on committing, for:

• moving enforce_max_args, if @ifreund_freebsdfoundation.org thinks it's reasonable to do so
• @kevans to have an opportunity to comment

[ifreund_freebsdfoundation.org]

Move all enforce_max_args() calls to start of function

[ifreund_freebsdfoundation.org]

libexec/flua/modules/lposix.c

154–155

Indeed, good catch. We should definitely invoke a lua error in the case of too many arguments being passed rather than return an error value.

Or - perhaps we can just check for too many args first in each fn?

We could definitely go this route, it would make this code more consistent/readable IMO for each function to declare up-front the maximum number of args it accepts. It also seems like the less error-prone approach.

My original reasoning for doing things in this order was to prefer giving the most specific error message possible rather than a generic "too many arguments." I'm no longer convinced that that original goal makes sense though or is worth the readability tradeoff.

[kevans]

libexec/flua/modules/lposix.c

52

Just noting that this is correct to be a maximum (rather than the minimum that was checked for before), based on observation:

> lgen.basename("/tmp/foo", "/tmp/bar")
stdin:1: bad argument #2 to 'basename' (no more than 1 argument expected, got 2)
stack traceback:
	[C]: in function 'posix.libgen.basename'
	stdin:1: in main chunk
	[C]: in ?