Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 6 Feb 2012 23:17:43 +0100
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Mikolaj Golub <trociny@freebsd.org>
Cc:        svn-src-head@FreeBSD.org, Guy Helmer <ghelmer@palisadesystems.com>, svn-src-all@FreeBSD.org, Andrey Zonov <andrey@zonov.org>, src-committers@FreeBSD.org
Subject:   Re: svn commit: r230869 - head/usr.sbin/daemon
Message-ID:  <20120206221742.GA1336@garage.freebsd.pl>
In-Reply-To: <86wr7zmy8f.fsf@kopusha.home.net>
References:  <20120204074201.GA1694@garage.freebsd.pl> <4F2CEB1D.10607@zonov.org> <27A0A960-F767-4D2C-BF3E-31F73FBF4E28@palisadesystems.com> <86zkcy5ur9.fsf@kopusha.home.net> <20120205093938.GC30033@garage.freebsd.pl> <86lioh7yz5.fsf@kopusha.home.net> <20120205214647.GI30033@garage.freebsd.pl> <86sjiov29o.fsf@in138.ua3> <20120206082706.GA1324@garage.freebsd.pl> <86wr7zmy8f.fsf@kopusha.home.net>

next in thread | previous in thread | raw e-mail | index | archive | help

--k+w/mQv8wyuph6w0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 06, 2012 at 11:46:24PM +0200, Mikolaj Golub wrote:
>=20
> On Mon, 6 Feb 2012 09:27:06 +0100 Pawel Jakub Dawidek wrote:
>=20
>  PJD> For the patch itself.
>=20
>  PJD> You don't have to have two separate cases depending on request for
>  PJD> pidfile. You can specify NULL pfh to the pidfile functions.
>  PJD> Even in example from the manual page when pfh is NULL there is a ca=
se
>  PJD> where we warn, but continue execution and call pidfile functions.
>  PJD> This should simplify the code.
>=20
>  PJD> If you do that (actually even if you don't), remember to either use
>  PJD> warn(3) before pidfile_remove(3) and exit(3) after or preserve errno
>  PJD> before calling pidfile_remove(3), as pidfile_remove(3) can modify i=
t if
>  PJD> unlink(2) is unsuccessful or pfh is NULL.
>=20
> Thanks. The updated version is attached.

Looks good to me.

> Index: usr.sbin/daemon/daemon.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- usr.sbin/daemon/daemon.c	(revision 231014)
> +++ usr.sbin/daemon/daemon.c	(working copy)
> @@ -32,6 +32,7 @@
>  __FBSDID("$FreeBSD$");
> =20
>  #include <sys/param.h>
> +#include <sys/wait.h>
> =20
>  #include <err.h>
>  #include <errno.h>
> @@ -43,15 +44,16 @@ __FBSDID("$FreeBSD$");
>  #include <unistd.h>
> =20
>  static void restrict_process(const char *);
> +static void wait_child(pid_t pid);
>  static void usage(void);
> =20
>  int
>  main(int argc, char *argv[])
>  {
>  	struct pidfh *pfh =3D NULL;
> -	int ch, nochdir, noclose, errcode;
> +	int ch, nochdir, noclose;
>  	const char *pidfile, *user;
> -	pid_t otherpid;
> +	pid_t otherpid, pid;
> =20
>  	nochdir =3D noclose =3D 1;
>  	pidfile =3D user =3D NULL;
> @@ -79,9 +81,7 @@ main(int argc, char *argv[])
>  	if (argc =3D=3D 0)
>  		usage();
> =20
> -	if (user !=3D NULL)
> -		restrict_process(user);
> -
> +	pfh =3D NULL;
>  	/*
>  	 * Try to open the pidfile before calling daemon(3),
>  	 * to be able to report the error intelligently
> @@ -100,22 +100,36 @@ main(int argc, char *argv[])
>  	if (daemon(nochdir, noclose) =3D=3D -1)
>  		err(1, NULL);
> =20
> -	/* Now that we are the child, write out the pid */
> -	if (pidfile)
> +	pid =3D 0;
> +	if (pidfile) {
> +		/*
> +		 * Spawn a child to exec the command, so in the parent
> +		 * we could wait for it to exit and remove pidfile.
> +		 */
> +		pid =3D fork();
> +		if (pid =3D=3D -1) {
> +			pidfile_remove(pfh);
> +			err(1, "fork");
> +		}
> +	}
> +	if (pid =3D=3D 0) {
> +		/* Now that we are the child, write out the pid. */
>  		pidfile_write(pfh);
> =20
> -	execvp(argv[0], argv);
> +		if (user !=3D NULL)
> +			restrict_process(user);
> =20
> -	/*
> -	 * execvp() failed -- unlink pidfile if any, and
> -	 * report the error
> -	 */
> -	errcode =3D errno; /* Preserve errcode -- unlink may reset it */
> -	if (pidfile)
> -		pidfile_remove(pfh);
> +		execvp(argv[0], argv);
> =20
> -	/* The child is now running, so the exit status doesn't matter. */
> -	errc(1, errcode, "%s", argv[0]);
> +		/*
> +		 * execvp() failed -- report the error. The child is
> +		 * now running, so the exit status doesn't matter.
> +		 */
> +		err(1, "%s", argv[0]);
> +	}
> +	wait_child(pid);
> +	pidfile_remove(pfh);
> +	exit(0); /* Exit status does not metter. */
>  }
> =20
>  static void
> @@ -132,6 +146,19 @@ restrict_process(const char *user)
>  }
> =20
>  static void
> +wait_child(pid_t pid)
> +{
> +	int status;
> +
> +	while (waitpid(pid, &status, 0) =3D=3D -1) {
> +		if (errno !=3D EINTR) {
> +			warn("waitpid");
> +			break;
> +		}
> +	}
> +}
> +
> +static void
>  usage(void)
>  {
>  	(void)fprintf(stderr,

--=20
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl

--k+w/mQv8wyuph6w0
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)

iEYEARECAAYFAk8wUYYACgkQForvXbEpPzRWFACeLMppI61xkOQjJ2nLUfGW5lYq
OvgAn1WBq/gfLvymZ3dfAehtk7OVhFjX
=ffH+
-----END PGP SIGNATURE-----

--k+w/mQv8wyuph6w0--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120206221742.GA1336>