Date: Sat, 14 Sep 2013 12:00:17 +0300 From: Mikolaj Golub <trociny@FreeBSD.org> To: John-Mark Gurney <jmg@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r255521 - head/usr.sbin/daemon Message-ID: <20130914090016.GA27774@gmail.com> In-Reply-To: <201309131657.r8DGvSnd020061@svn.freebsd.org> References: <201309131657.r8DGvSnd020061@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Sep 13, 2013 at 04:57:28PM +0000, John-Mark Gurney wrote: > Author: jmg > Date: Fri Sep 13 16:57:28 2013 > New Revision: 255521 > URL: http://svnweb.freebsd.org/changeset/base/255521 > > Log: > add support for writing the pid of the daemon program to a pid file so > that daemon can be used w/ rc.subr and ports can use the additional > functionality, such as keeping the ldap daemon up and running, and have > the proper program to signal to exit.. > > PR: bin/181341 > Submitted by: feld > Approved by: re (glebius) Thank you for doing this. I was aware about this issue after I had added restart option but could not come with a solution that would satisfy me. Having 2 options for pid files might look a little confusing but I think it is much better than we had before. Still please see a couple of notes below. > Modified: head/usr.sbin/daemon/daemon.c > ============================================================================== > --- head/usr.sbin/daemon/daemon.c Fri Sep 13 14:15:52 2013 (r255520) > +++ head/usr.sbin/daemon/daemon.c Fri Sep 13 16:57:28 2013 (r255521) > @@ -53,16 +53,17 @@ static void usage(void); > int > main(int argc, char *argv[]) > { > - struct pidfh *pfh = NULL; > + struct pidfh *ppfh, *pfh; > sigset_t mask, oldmask; > int ch, nochdir, noclose, restart; > - const char *pidfile, *user; > + const char *pidfile, *ppidfile, *user; > pid_t otherpid, pid; > > nochdir = noclose = 1; > restart = 0; > - pidfile = user = NULL; > - while ((ch = getopt(argc, argv, "cfp:ru:")) != -1) { > + ppfh = pfh = NULL; > + ppidfile = pidfile = user = NULL; > + while ((ch = getopt(argc, argv, "cfp:P:ru:")) != -1) { > switch (ch) { > case 'c': > nochdir = 0; > @@ -73,6 +74,9 @@ main(int argc, char *argv[]) > case 'p': > pidfile = optarg; > break; > + case 'P': > + ppidfile = optarg; > + break; > case 'r': > restart = 1; > break; > @@ -89,7 +93,7 @@ main(int argc, char *argv[]) > if (argc == 0) > usage(); > > - pfh = NULL; > + ppfh = pfh = NULL; > /* > * Try to open the pidfile before calling daemon(3), > * to be able to report the error intelligently > @@ -104,6 +108,18 @@ main(int argc, char *argv[]) > err(2, "pidfile ``%s''", pidfile); > } > } > + > + /* do same for actual daemon process */ > + if (ppidfile != NULL) { > + ppfh = pidfile_open(ppidfile, 0600, &otherpid); > + if (ppfh == NULL) { > + if (errno == EEXIST) { > + errx(3, "process already running, pid: %d", > + otherpid); > + } > + err(2, "ppidfile ``%s''", ppidfile); You have to pidfile_remove(pfh) before exiting not to leave the child pidfile on fs. Also pidfile_remove(ppfh) if fork() fails, and there are several other places where pidfile_remove() is missed for the child too before err exit (me should be blamed for most of these). > + } > + } > > if (daemon(nochdir, noclose) == -1) > err(1, NULL); > @@ -176,12 +192,17 @@ restart: > */ > err(1, "%s", argv[0]); > } > + /* write out parent pidfile if needed */ > + if (ppidfile != NULL) > + pidfile_write(ppfh); > + No need to check for ppidfile != NULL. It is safe to call pidfile_write() (and pidfile_remove()) with NULL argument. Also, doing pidfile_write() here will cause needlessly rewriting the file after every child restart. I think it is better to call pidfile_write(ppfh) once, before the restart loop, just after daemon(). > setproctitle("%s[%d]", argv[0], pid); > if (wait_child(pid, &mask) == 0 && restart) { > sleep(1); > goto restart; > } > pidfile_remove(pfh); > + pidfile_remove(ppfh); > exit(0); /* Exit status does not matter. */ > } > > @@ -240,7 +261,7 @@ static void > usage(void) > { > (void)fprintf(stderr, > - "usage: daemon [-cfr] [-p pidfile] [-u user] command " > - "arguments ...\n"); > + "usage: daemon [-cfr] [-p child_pidfile] [-P supervisor_pidfile] " > + "[-u user]\n command arguments ...\n"); > exit(1); > } -- Mikolaj Golub
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130914090016.GA27774>