This matches the interface of lposix, although I do wonder why they went
with execp rather than execvp for the function name here.
Sponsored by: The FreeBSD Foundation
Details
Details
- Reviewers
emaste kevans - Commits
- rG7080a0c170da: flua: add posix.unistd.execp
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 64079 Build 60963: arc lint + arc unit
Event Timeline
| libexec/flua/modules/lposix.c | ||
|---|---|---|
| 212 | lots of magic numbers w/o explanation. I suspect they are related to the argc + 2 below, but am not sure. I suspect you'll be ample to explain. Also, how does flow control work here: it's an error for there not to be enough args, but you shouldn't continue processing... | |
| 235 | unhandled error path? | |
| libexec/flua/modules/lposix.c | ||
|---|---|---|
| 212 | The n == 2 argument is the condition checked for, the lua function implement requires exactly 2 arguments. The 3rd argument to luaL_argcheck() determines which argument to highlight in the lua error message in the case of an error. lua_chmod() in this file has exactly the same logic for example, perhaps we could add a more readable abstraction here. As for the control flow question, luaL_argcheck() does not return in the case of an error (that is, if n == 2 is false here). It generates a lua error and longjmps away to elsewhere in the VM. | |
| 235 | luaL_argerror() never returns, it raises a lua error and longjmps away as explained above. | |
Comment Actions
Add comments to aid review
I realized that the reasoning for using lua_newuserdatauv() is not
immediately obvious unless one has experience working with this
style of language extension API. I'm sure someone will find this
comment useful in the future.
| libexec/flua/modules/lposix.c | ||
|---|---|---|
| 219–220 | style(9) puts a * at the beginning of each line of the comment | |
Comment Actions
Implementation looks fine to me; it does make me wish that we'd settle on an appropriate test framework for lua, though. I'd love to have a test that confirms that the argv[0] behavior works as expected (and doesn't, e..g, crash when it's omitted). I don't insist on trying to solve that kind of problem before proceeding here, though... just something to think about.
| libexec/flua/modules/lposix.c | ||
|---|---|---|
| 251 | Is there a chance that lua_pushnil could've clobbered errno before we use it here? | |
| libexec/flua/modules/lposix.c | ||
|---|---|---|
| 251 | I'm pretty sure that's not possible but I would also feel more comfortable storing errno before calling into lua, I'll add a variable here. | |