From owner-svn-src-all@FreeBSD.ORG Mon Feb 6 22:19:00 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0979A106566B; Mon, 6 Feb 2012 22:19:00 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (60.wheelsystems.com [83.12.187.60]) by mx1.freebsd.org (Postfix) with ESMTP id 3F10F8FC1B; Mon, 6 Feb 2012 22:18:58 +0000 (UTC) Received: from localhost (89-73-195-149.dynamic.chello.pl [89.73.195.149]) by mail.dawidek.net (Postfix) with ESMTPSA id 06FF9C04; Mon, 6 Feb 2012 23:18:56 +0100 (CET) Date: Mon, 6 Feb 2012 23:17:43 +0100 From: Pawel Jakub Dawidek To: Mikolaj Golub Message-ID: <20120206221742.GA1336@garage.freebsd.pl> 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="k+w/mQv8wyuph6w0" Content-Disposition: inline In-Reply-To: <86wr7zmy8f.fsf@kopusha.home.net> X-OS: FreeBSD 10.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@FreeBSD.org, Guy Helmer , svn-src-all@FreeBSD.org, Andrey Zonov , src-committers@FreeBSD.org Subject: Re: svn commit: r230869 - head/usr.sbin/daemon X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Feb 2012 22:19:00 -0000 --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 > +#include > =20 > #include > #include > @@ -43,15 +44,16 @@ __FBSDID("$FreeBSD$"); > #include > =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--