On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> static void check_dead_children(void)
> {
> - int status;
> - pid_t pid;
> -
> - while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
> - const char *dead = "";
> - remove_child(pid);
> - if (!WIFEXITED(status) || (WEXITSTATUS(status) > 0))
> - dead = " (with error)";
> - loginfo("[%"PRIuMAX"] Disconnected%s", (uintmax_t)pid, dead);
> - }
> + struct child **cradle, *blanket;
> + for (cradle = &firstborn; (blanket = *cradle);)
> + if (!is_async_alive(&blanket->async)) {
This would be the right place to call finish_async(). But since we cannot
wait, you invented is_async_alive(). But actually we are not only interested
in whether the process is alive, but also whether it completed successfully
so that we can add "(with error)". Would it make sense to have a function
finish_async_nowait() instead of is_async_alive() that (1) stresses the
start/finish symmetry and (2) can return more than just Boolean?
> + *cradle = blanket->next;
> + loginfo("Disconnected\n");
Here you are losing information about the pid, which is important to have in
the syslog. The \n should be dropped.
> + async.proc = async_execute;
> + async.data = ss;
> + async.out = incoming;
>
> - dup2(incoming, 0);
> - dup2(incoming, 1);
> + if (start_async(&async))
> + logerror("unable to fork");
> + else
> + add_child(&async, addr, addrlen);
> close(incoming);
> -
> - exit(execute(0, addr));
In start_command(), the convention is that fds that are provided by the caller
are closed by start_command() (even if there are errors). The close(incoming)
that you leave here indicates that you are not using the same convention with
start_async(). It would be nice to switch to the same convention.
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html