Re: [PATCH 2/5] daemon: if one of the standard fds is missing open it to /dev/null

Edgar Toernig [off-list ref] wrote:

Matthias Lederhofer wrote:

quoted

+/* if any standard file descriptor is missing open it to /dev/null */
+static void sanitize_stdfds(void)
+{
+	int devnull = -1, i;
+	struct stat buf;
+	for (i = 0; i < 3; ++i) {
+		if (fstat(i, &buf) != -1)
+			continue;
+		if (devnull == -1 &&
+			(devnull = open("/dev/null", O_RDWR, 0)) == -1)
+			die("open /dev/null failed: %s", strerror(errno));
+		if (dup2(devnull, i) != i)
+			die("dup2 failed: %s", strerror(errno));
+	}
+	if (devnull != -1)
+		close(devnull);
+}

This looks broken.  The open will return i as this is
the lowest free fd.  I don't know what POSIX says
about dup2(i,i) but anyway, you close it at the end
which completely defeats the intent of the function.

How's this?

	devnull = open("/dev/null", O_RDWR, 0);
	if (devnull == 0)
		devnull = dup(devnull);
	if (devnull == 1)
		devnull = dup(devnull);
	if (devnull == -1)
		die("open/dup /dev/null failed: %s", strerror(errno));
	if (devnull > 2)
		close(devnull);

You're right (also for the daemonize function to use sanitize_stdfds).
The code looks good to me, this could also be done using a while-loop
(making it a little bit shorter, I don't know what is easier to read):

    devnull = open("/dev/null", O_RDWR, 0);
    while (devnull != -1 && devnull < 2)
        dup(devnull);
    if (devnull == -1)
        die("..");
    close(devnull);

(This is similar to what Andre Noll posted.)

I'll correct and resend those patches later.