Age | Commit message (Collapse) | Author |
|
Thanks to https://github.com/include-what-you-use/include-what-you-use
|
|
A short history lesson: this code (from exec.c) used to just be
write(...);
In 649d85b, an empty if block was added to placate _FORTIFY_SOURCE's
warn_unused_result:
if (write(...) != sizeof(error)) {
// ...
}
This is fine since the parent doesn't rely on receiving those writes
(e.g. the child could die before writing), and the small write to a pipe
won't fail anyway.
But when bfs_spawn() was created, the implementation changed to this:
while (write(...) < sizeof(error));
The while loop was presumably added to mirror musl's posix_spawn()
implementation, which handles EINTR. But musl's is actually correct:
while (write(...) < 0);
whereas mine has a subtle bug: since sizeof(error) is unsigned, the bfs
code did an unsigned comparison, meaning -1 from write() would *not*
restart the loop.
Fix it by comparing with 0 like musl, since short writes are impossible
here. Perhaps it's time for -Wsign-compare, despite the other 18
instances being false positives.
|
|
We have to check in_fd after out_fd, otherwise the dup() may move it to
in_fd.
|
|
The old code could dup() the pipe from in_fd to out_fd, for example, and
neglected to keep it CLOEXEC.
|
|
|
|
|
|
|
|
Credit to https://github.com/nim-lang/Nim/issues/3138 for the idea to
just overwrite environ and call execvp() instead of duplicating the path
searching logic.
|
|
|