From owner-svn-src-all@FreeBSD.ORG Mon Feb 6 07:39:54 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 A9B51106564A; Mon, 6 Feb 2012 07:39:54 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-bk0-f54.google.com (mail-bk0-f54.google.com [209.85.214.54]) by mx1.freebsd.org (Postfix) with ESMTP id 6ED918FC15; Mon, 6 Feb 2012 07:39:53 +0000 (UTC) Received: by bkbzx1 with SMTP id zx1so6141252bkb.13 for ; Sun, 05 Feb 2012 23:39:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=from:to:cc:subject:organization:references:sender:date:in-reply-to :message-id:user-agent:mime-version:content-type; bh=V9qFGYI00/KfxgwblkohphIHs3hT2fmugMeHB6NixuY=; b=BVvYVNqcUOGmraH1tiH6eWEOjp5vN3wXQ9K3wSzBfflqXoqpBL5UdjYse63wxb1kuF wmSEctwWmyoPe8c8xkC8yUfX3+l94EWWGDni3yE7EmV5Fz8j3ubEnTMQZee8NwZ5LwC9 X0B6oKyKg9OzgDXiqwTtNK491NegnfLWWh/ew= Received: by 10.204.173.11 with SMTP id n11mr7904131bkz.120.1328513992428; Sun, 05 Feb 2012 23:39:52 -0800 (PST) Received: from localhost ([94.27.39.186]) by mx.google.com with ESMTPS id sp6sm42640572bkb.2.2012.02.05.23.39.49 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 05 Feb 2012 23:39:50 -0800 (PST) From: Mikolaj Golub To: Pawel Jakub Dawidek Organization: TOA Ukraine References: <201202011641.q11Gf0j6095461@svn.freebsd.org> <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> Sender: Mikolaj Golub Date: Mon, 06 Feb 2012 09:39:47 +0200 In-Reply-To: <20120205214647.GI30033@garage.freebsd.pl> (Pawel Jakub Dawidek's message of "Sun, 5 Feb 2012 22:46:48 +0100") Message-ID: <86sjiov29o.fsf@in138.ua3> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" 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 07:39:54 -0000 --=-=-= On Sun, 5 Feb 2012 22:46:48 +0100 Pawel Jakub Dawidek wrote: PJD> On Sun, Feb 05, 2012 at 11:27:10PM +0200, Mikolaj Golub wrote: >> Ok, using hastd code as a reference :-) here is my implementation. PJD> - I'd not pass selected signals to the child. The parent can still be PJD> killed with a whole bunch of different signals that are not passed or PJD> cannot be caught or the child process handle them gracefully. PJD> Signals should be send to the PID from the pidfile anyway. If someone PJD> is sending signals to the parent he has no right to expect well PJD> behaviour from the parent. Well, sending a whole bunch of different signals to parent we might not expect right behavior, but why not to provide it for the "standard" ones? E.g. on shutdown init(8) will send SIGTERM and the daemon will gracefully exit terminating the child and cleaning up the pidfile. If the the child process does not handle SIGTERM gracefully I don't see much difference from having only this one process alive or two (with its monitoring daemon). The pidfile is seen in ps(1) output for the daemon process, which allows to identify the monitoring daemon with its child. Or we could change its proctitle to something like "daemon: cmdname[pid]", similar to what sshd does. So people would expect that terminating a daemon will terminate the process it monitors. PJD> - Now that we handle the pidfile fully in the parent, I'd move dropping PJD> provileges after fork(2) and pidfile_write(3). This way pidfiles will PJD> always be created with root privileges and we can forget about all the PJD> mess with pid directories, etc. PJD> - With the above you can wait for child to exit with simple wait(2). Yes, it looks like much simpler, see the attached patch. But I don't think I like it much as it still looks like a half measure to me. -- Mikolaj Golub --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=daemon.c.patch Index: usr.sbin/daemon/daemon.c =================================================================== --- usr.sbin/daemon/daemon.c (revision 231060) +++ usr.sbin/daemon/daemon.c (working copy) @@ -32,6 +32,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include @@ -49,9 +50,9 @@ int main(int argc, char *argv[]) { struct pidfh *pfh = NULL; - int ch, nochdir, noclose, errcode; + int ch, nochdir, noclose, status; const char *pidfile, *user; - pid_t otherpid; + pid_t otherpid, pid; nochdir = noclose = 1; pidfile = user = NULL; @@ -79,43 +80,61 @@ main(int argc, char *argv[]) if (argc == 0) usage(); - if (user != NULL) - restrict_process(user); + if (pidfile == NULL) { + /* + * This is a simple case. Daemonize and exec. + */ + if (daemon(nochdir, noclose) == -1) + err(1, NULL); + if (user != NULL) + restrict_process(user); + + execvp(argv[0], argv); + + /* + * execvp() failed -- report the error. The child is + * now running, so the exit status doesn't matter. + */ + err(1, "%s", argv[0]); + } + /* * Try to open the pidfile before calling daemon(3), * to be able to report the error intelligently */ - if (pidfile) { - pfh = pidfile_open(pidfile, 0600, &otherpid); - if (pfh == NULL) { - if (errno == EEXIST) { - errx(3, "process already running, pid: %d", - otherpid); - } - err(2, "pidfile ``%s''", pidfile); + pfh = pidfile_open(pidfile, 0600, &otherpid); + if (pfh == NULL) { + if (errno == EEXIST) { + errx(3, "process already running, pid: %d", + otherpid); } + err(2, "pidfile ``%s''", pidfile); } if (daemon(nochdir, noclose) == -1) err(1, NULL); - /* Now that we are the child, write out the pid */ - if (pidfile) + pid = fork(); + if (pid == -1) { + pidfile_remove(pfh); + err(1, "fork"); + } + if (pid == 0) { + /* Now that we are the child, write out the pid. */ pidfile_write(pfh); - execvp(argv[0], argv); + if (user != NULL) + restrict_process(user); - /* - * execvp() failed -- unlink pidfile if any, and - * report the error - */ - errcode = errno; /* Preserve errcode -- unlink may reset it */ - if (pidfile) - pidfile_remove(pfh); + execvp(argv[0], argv); - /* The child is now running, so the exit status doesn't matter. */ - errc(1, errcode, "%s", argv[0]); + /* execvp() failed. */ + err(1, "%s", argv[0]); + } + (void)wait(&status); + pidfile_remove(pfh); + exit(0); } static void --=-=-=--