start-stop-daemon incorrect pid
Stephane Billiart
stephane.billiart at gmail.com
Thu Sep 7 13:46:27 UTC 2006
On 06/09/06 ? ? 21:02, Rob Landley wrote:
> On Wednesday 06 September 2006 3:46 pm, Stephane Billiart wrote:
> > On 06/09/06 ? ? 11:11, Natanael Copa wrote:
> > > Creating pids in several applets sounds like bloat.
> >
> > I've already posted this patch I've been using for months on my system
> > to create pidfiles for monitoring purposes but it never made it to the
> > SVN tree.
> >
> > I split the patch in two parts, the first adds writepidfile/removepidfile
> > to libbb and the second modifies inetd, syslogd and crond to use it.
> > fakeidentd is another daemon that could use this but it also does a
> > fchown on the pidfile so I did not modify it here (I don't use it anyway)
>
> Looks reasonable, except:
>
> 1) Please use your own copyright notice. We're not the FSF, you don't assign
> copyright to us. And legally assigning copyright isn't trivial, which is one
> of the issues in the SCO vs Novell lawsuit. A decent summary is here:
>
> http://www.groklaw.net/article.php?story=20040326223857634
>
> Putting Erik's copyright notice on it is not an "instrument of conveyance",
> and if we go to enforce copyrights on a joint work it would SUCK for the
> other side to be able to point out examples showing that who owns what is
> unclear.
>
> (Note that we don't always list every copyright in the files, especially when
> we add a small chunk to a large existing file. Instead we mention the
> contributor in the SVN checkin comment for that commit. I just don't want
> there to be _false_ information in there.)
>
> 2) We use the short GPL boilerplate now.
OK, I corrected these.
I also added support for fakeidentd, now only udhcp uses its own pidfile
routines (with locking).
>
> 3) So what happens if the thing's already running? The libbb code doesn't
> check for an existing applet, which I thought was the point of a pidfile...?
pidfiles just make checking for a process presence/absence/death easier
in monitoring programs and startup/shutdown scripts.
Preventing a service to be launched several times is usually done in
those scripts, not in the daemon code itself.
I originally added the feature because I use monit
http://www.tildeslash.com/monit/
and it requires pidfiles to do its job.
New patches attached.
--
Stéphane Billiart http://perso.orange.fr/billiart/
-------------- next part --------------
Index: libbb/Makefile.in
===================================================================
--- libbb/Makefile.in (revision 16063)
+++ libbb/Makefile.in (working copy)
@@ -22,7 +22,7 @@
kernel_version.c last_char_is.c login.c \
make_directory.c md5.c mode_string.c mtab_file.c \
obscure.c parse_mode.c parse_number.c perror_msg.c \
- perror_msg_and_die.c get_console.c \
+ perror_msg_and_die.c pidfile.c get_console.c \
process_escape_sequence.c procps.c \
recursive_action.c remove_file.c \
restricted_shell.c run_parts.c run_shell.c safe_read.c safe_write.c \
Index: libbb/pidfile.c
===================================================================
--- libbb/pidfile.c (revision 0)
+++ libbb/pidfile.c (revision 0)
@@ -0,0 +1,25 @@
+/* vi: set sw=4 ts=4: */
+/*
+ * pid file routines
+ *
+ * Copyright (C) 2006 by Stephane Billiart <stephane.billiart at gmail.com>
+ *
+ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
+ */
+#include <stdio.h>
+#include <errno.h>
+#include "libbb.h"
+
+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path)
+{
+ FILE *pidf;
+
+ if ((pidf = bb_wfopen(path, "w")) != NULL) {
+ fprintf(pidf, "%u\n", getpid());
+ fclose(pidf);
+ return 0;
+ }
+ return 1;
+}
+#endif
Index: include/libbb.h
===================================================================
--- include/libbb.h (revision 16063)
+++ include/libbb.h (working copy)
@@ -120,6 +120,13 @@
};
extern int logmode;
+#ifdef CONFIG_FEATURE_PIDFILE
+extern int writepidfile(const char *path);
+#define removepidfile(f) remove_file(f, FILEUTILS_FORCE)
+#else
+#define writepidfile(f) ((void)0)
+#define removepidfile(f) ((void)0)
+#endif
extern void bb_show_usage(void) ATTRIBUTE_NORETURN ATTRIBUTE_EXTERNALLY_VISIBLE;
extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2)));
extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, format (printf, 1, 2)));
Index: Config.in
===================================================================
--- Config.in (revision 16063)
+++ Config.in (working copy)
@@ -135,6 +135,12 @@
Don't enable this unless you have a really good reason to clean
things up manually.
+config CONFIG_FEATURE_PIDFILE
+ bool "Support for pidfile writing"
+ default n
+ help
+ Enable code to write pidfiles (crond and syslogd)
+
config CONFIG_FEATURE_SUID
bool "Support for SUID/SGID handling"
default n
-------------- next part --------------
Index: networking/fakeidentd.c
===================================================================
--- networking/fakeidentd.c (revision 16063)
+++ networking/fakeidentd.c (working copy)
@@ -102,31 +102,10 @@
static void handlexitsigs(int signum)
{
- if (unlink(PIDFILE) < 0)
- close(open(PIDFILE, O_WRONLY|O_CREAT|O_TRUNC, 0644));
+ removepidfile(PIDFILE);
exit(0);
}
-/* May succeed. If not, won't care. */
-static void writepid(uid_t nobody, uid_t nogrp)
-{
- char buf[24];
- int fd = open(PIDFILE, O_WRONLY|O_CREAT|O_TRUNC, 0664);
-
- if (fd < 0)
- return;
-
- snprintf(buf, 23, "%d\n", getpid());
- write(fd, buf, strlen(buf));
- fchown(fd, nobody, nogrp);
- close(fd);
-
- /* should this handle ILL, ... (see signal(7)) */
- signal(SIGTERM, handlexitsigs);
- signal(SIGINT, handlexitsigs);
- signal(SIGQUIT, handlexitsigs);
-}
-
/* return 0 as parent, 1 as child */
static int godaemon(void)
{
@@ -143,8 +122,15 @@
bb_error_msg_and_die("Cannot find uid/gid of user '%s'", nobodystr);
nobody = pw->pw_uid;
nogrp = pw->pw_gid;
- writepid(nobody, nogrp);
+ if (writepidfile(PIDFILE)) {
+ chown(PIDFILE, nobody, nogrp);
+ /* should this handle ILL, ... (see signal(7)) */
+ signal(SIGTERM, handlexitsigs);
+ signal(SIGINT, handlexitsigs);
+ signal(SIGQUIT, handlexitsigs);
+ }
+
close(0);
inetbind();
xsetgid(nogrp);
Index: networking/inetd.c
===================================================================
--- networking/inetd.c (revision 16063)
+++ networking/inetd.c (working copy)
@@ -1191,7 +1191,7 @@
}
(void) close (sep->se_fd);
}
- (void) unlink (_PATH_INETDPID);
+ removepidfile (_PATH_INETDPID);
exit (0);
}
@@ -1294,14 +1294,7 @@
/* If run by hand, ensure groups vector gets trashed */
setgroups (1, &gid);
}
-
- {
- FILE *fp;
-
- if ((fp = fopen (_PATH_INETDPID, "w")) != NULL) {
- fprintf (fp, "%u\n", getpid ());
- (void) fclose (fp);
- }
+ writepidfile (_PATH_INETDPID);
}
if (getrlimit (RLIMIT_NOFILE, &rlim_ofile) < 0) {
Index: sysklogd/syslogd.c
===================================================================
--- sysklogd/syslogd.c (revision 16063)
+++ sysklogd/syslogd.c (working copy)
@@ -425,6 +425,7 @@
unlink(lfile);
if (ENABLE_FEATURE_IPC_SYSLOG)
ipcsyslog_cleanup();
+ removepidfile("/var/run/syslogd.pid");
exit(TRUE);
}
@@ -641,6 +642,7 @@
xdaemon(0, 1);
#endif
}
+ writepidfile("/var/run/syslogd.pid");
doSyslogd();
return EXIT_SUCCESS;
Index: miscutils/crond.c
===================================================================
--- miscutils/crond.c (revision 16063)
+++ miscutils/crond.c (working copy)
@@ -216,6 +216,7 @@
int rescan = 60;
short sleep_time = 60;
+ writepidfile("/var/run/cron.pid");
for (;;) {
sleep((sleep_time + 1) - (short) (time(NULL) % sleep_time));
More information about the busybox
mailing list