Same as in glibc
Details
Details
- Reviewers
kevans rwatson ngie - Commits
- rG78963d796d5c: Document posix_spawn_file_actions_addclosefrom_np(3)
rGf16ec9c6e36f: Add tests for posix_spawn_file_actions_add{chdir,fchdir}_np(3)
rGa18ddf775744: posix_spawn: add closefrom non-portable action
rGbd44dce5b317: Document posix_spawn_file_actions_addchdir_np(3)
rG25cda42a4981: posix_spawn: add chdir-related non-portable actions
rG8ed1e4a5c1a9: posix_spawn(3), posix_spawn_file_actions_addopen(3): use .Fo/.Fc
rGb239cc204a57: posix_spawn: style, use return ()
rGadbaf1b443c6: posix_spawn.c: format fae_action anon enum more stylish
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Tests look good! Thank you 💚!
All of my comments are about best practices with atf(4) and other related questions/nits.
| contrib/netbsd-tests/lib/libc/gen/posix_spawn/t_fileactions.c | ||
|---|---|---|
| 437 ↗ | (On Diff #99095) | Non-blocking nit: make /bin/pwd a constant, e.g., PWD. I looked up _PATH_PWD and it seems that it's already taken by pwd.h . |
| include/spawn.h | ||
| 95 ↗ | (On Diff #99095) | Nit: space between np and (. |
| lib/libc/gen/posix_spawn_file_actions_addopen.3 | ||
| 260 ↗ | (On Diff #99095) | Non-blocking comment: is there a macro that could be used along with mentioning glibc? Also, should it glibc or GNU libc? |
| contrib/netbsd-tests/lib/libc/gen/posix_spawn/t_fileactions.c | ||
|---|---|---|
| 398 ↗ | (On Diff #99095) | This breaks the ATF sandbox: it's better to use the current test directory and nest underneath it to ensure that atf can clean up all files leftover after a test run. |
| 420–428 ↗ | (On Diff #99095) | This could be done with atf_utils_wait instead. |
| contrib/netbsd-tests/lib/libc/gen/posix_spawn/t_fileactions.c | ||
|---|---|---|
| 398 ↗ | (On Diff #99095) | In which way it breaks? The directory is used to chdir() into it, nothing more. There are no state stored in /tmp |
| 420–428 ↗ | (On Diff #99095) | I do not understand what you are proposing to do there. |
| lib/libc/gen/posix_spawn_file_actions_addopen.3 | ||
| 260 ↗ | (On Diff #99095) | I do not think we have or would ever have such macro. Official GNU project is called The GNU C Library (glibc), and they call themself glibc. |
| contrib/netbsd-tests/lib/libc/gen/posix_spawn/t_fileactions.c | ||
|---|---|---|
| 398 ↗ | (On Diff #99095) | Ah, got it. Yeah, it's not an issue then (only if files, etc, were created there). The whole thing about the ATF sandbox is to make sure that ATF can clean up all files/directories created by test cases. If a test case creates something outside that hierarchy, then there will be stale data kicking around the test host which can result in non-deterministic test behavior or resource utilization over time that could cause minor grief for the host running the test, requiring manual intervention from a sysadmin. |
| 420–428 ↗ | (On Diff #99095) | atf_utils_wait would eliminate the need for explicitly calling waitpid, fread, etc. |
| contrib/netbsd-tests/lib/libc/gen/posix_spawn/t_fileactions.c | ||
|---|---|---|
| 420–428 ↗ | (On Diff #99095) | atf_utils_wait() requires atf_utils_fork(), from what I see. It cannot be used to get stdout from arbitrary spawned child. And the whole point of this test is that it spawns the process using specific posix_spawn() invocation. |
| lib/libc/gen/Symbol.map | ||
|---|---|---|
| 439 ↗ | (On Diff #99124) | Maybe re-sort FBSD_1.7 pre-commit? I note, e.g.. FBSD_1.2 is sorted despite splitting up *utx*symbols. I don't feel strongly about it, though. |