From mboxrd@z Thu Jan  1 00:00:00 1970
From: Muli Ben-Yehuda <muli@il.ibm.com>
Subject: Re: [PATCH] Fix bug #709 by daemonizing blktapctrl and
	closing stdin, stdout and stderr
Date: Mon, 31 Jul 2006 20:31:58 +0300
Message-ID: <20060731173158.GA5587@rhun.ibm.com>
References: <1154358942.7720.15.camel@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <xen-devel-bounces@lists.xensource.com>
Content-Disposition: inline
In-Reply-To: <1154358942.7720.15.camel@localhost.localdomain>
List-Unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
Cc: andrew.warfield@cl.cam.ac.uk, xen-devel@lists.xensource.com, julian.chesterfield@cl.cam.ac.uk
List-Id: xen-devel@lists.xenproject.org

On Mon, Jul 31, 2006 at 04:15:42PM +0100, Harry Butterworth wrote:

> +static void daemonize(void)
> +{
> +	pid_t pid;
> +
> +	/* Separate from our parent via fork, so init inherits us. */
> +	if ((pid = fork()) < 0)
> +		DPRINTF("Failed to fork daemon\n");

It will be useful to know why fork() failed (i.e., print errno
directly or use sterror_r() and friends).

> +	if (pid != 0)
> +		exit(0);

If fork() failed, this will cause us to exit(0) which doesn't seem
particularly appropriate.

> +	/* Session leader so ^C doesn't whack us. */
> +	setsid();

In theory setsid() can fail.

> +	/* Let session leader exit so child cannot regain CTTY */
> +	if ((pid = fork()) < 0)
> +		DPRINTF("Failed to fork daemon\n");
> +	if (pid != 0)
> +		exit(0);

Same comment as above.
> +
> +#ifndef TESTING	/* Relative paths for socket names */
> +	/* Move off any mount points we might be in. */
> +	if (chdir("/") == -1)
> +		DPRINTF("Failed to chdir\n");
> +#endif
> +	/* Discard our parent's old-fashioned umask prejudices. */
> +	umask(0);
> +
> +        close(STDIN_FILENO);

Mixed tabs and spaces, ugh. Also, fileno(stdin) is nicer.

Cheers,
Muli