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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
