From owner-svn-src-head@FreeBSD.ORG Sat Sep 14 09:00:22 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id E3607448; Sat, 14 Sep 2013 09:00:22 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-ee0-x22d.google.com (mail-ee0-x22d.google.com [IPv6:2a00:1450:4013:c00::22d]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 0F3B82A72; Sat, 14 Sep 2013 09:00:21 +0000 (UTC) Received: by mail-ee0-f45.google.com with SMTP id c50so1015967eek.4 for ; Sat, 14 Sep 2013 02:00:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=j9QTl6GKcFEgkWI1wZfAJIJm0yH16HVzSh1Fl0oDF8c=; b=GOQ25WQp1tqFr63xHFOvJNgII7v6Pc4PQ0po9IWoSLC8cKpvFdNNMoMeTUEoVi/zI/ W1uqESNj+3J9EwDNXja6p8t9UiBSKac0Tl4AMQYxCGVMUyV0ZUWz5QoCFliZrL9UPUKJ OTc1X07hIke0JvKepddalTdYUHbVLlj3LgmOlWl2EFjbxdB2q+cznEg4RGEuA2f4NOq5 AsFA7AfZ401w+ueCXinIEcQKxoicGByA1GTdpDigS2oE8ScLYsjNMy91yBhJW5yzUt9s PyNX8ipIGD+43d8XGKbecnYS05YntfxAXqQx3IV+9GrFWai8Yh7fLctYXngKy1NKant2 3ApA== X-Received: by 10.15.45.8 with SMTP id a8mr24970015eew.1.1379149220368; Sat, 14 Sep 2013 02:00:20 -0700 (PDT) Received: from localhost ([178.150.115.244]) by mx.google.com with ESMTPSA id h52sm22265326eez.3.1969.12.31.16.00.00 (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 14 Sep 2013 02:00:19 -0700 (PDT) Sender: Mikolaj Golub Date: Sat, 14 Sep 2013 12:00:17 +0300 From: Mikolaj Golub To: John-Mark Gurney Subject: Re: svn commit: r255521 - head/usr.sbin/daemon Message-ID: <20130914090016.GA27774@gmail.com> References: <201309131657.r8DGvSnd020061@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201309131657.r8DGvSnd020061@svn.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Sep 2013 09:00:23 -0000 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