From: Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
Subject: Re: [PATCH] Fix bug #709 by daemonizing blktapctrl and
	closing stdin, stdout and stderr
Date: Tue, 01 Aug 2006 11:01:36 +0100
Message-ID: <1154426496.7694.10.camel@localhost.localdomain>
References: <1154358942.7720.15.camel@localhost.localdomain>
	<20060731173158.GA5587@rhun.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
In-Reply-To: <20060731173158.GA5587@rhun.ibm.com>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Muli Ben-Yehuda <muli@il.ibm.com>
Cc: andrew.warfield@cl.cam.ac.uk, xen-devel@lists.xensource.com, julian.chesterfield@cl.cam.ac.uk

On Mon, 2006-07-31 at 20:31 +0300, Muli Ben-Yehuda wrote:
> 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);

OK so when I copied the code from xenstored_core.c I failed to notice
that barf_perror exits with an error code and DPRINTF which I replaced
it with doesn't.

That was pretty lame.

I'll fix these.

> > +
> > +        close(STDIN_FILENO);
> 
> Mixed tabs and spaces, ugh.

My mistake.

>  Also, fileno(stdin) is nicer.

Why? I don't think so.

> 
> Cheers,
> Muli