Hello Stephane,

Thanks for your efforts to improve busybox.

On Monday 26 March 2007 23:26, Stephane Billiart wrote:
> With the patch, people who don't need pidfiles at all, because they're
> using runsv or simply don't care can get a smaller busybox with no pidfile
> at all and those, like me, who want them can enable them so that crond
> and syslogd behave like the traditional versions.

+int writepidfile(const char *path)
+{
+       FILE *pidf;
+
+       pidf = fopen_or_warn(path, "w");
+       if (pidf) {
+               fprintf(pidf, "%u\n", getpid());
+               fclose(pidf);
+               return 0;
+       }
+       return 1;
+}

Can we avoid using stdio.h here? Pseudocode:
open();
full_write(itoa(getpid()) + "\n");
close();

But minor issues aside, let's take a look at this:

+#define removepidfile(f) remove_file(f, FILEUTILS_FORCE)

Please take a good look into remove_file() function body:

int remove_file(const char *path, int flags)
{
...
        if (S_ISDIR(path_stat.st_mode)) {
...
                if ((!(flags & FILEUTILS_FORCE) && access(path, W_OK) < 0 &&
                                        isatty(0)) ||
                                (flags & FILEUTILS_INTERACTIVE)) {
                        fprintf(stderr, "%s: descend into directory '%s'? ", 
applet_name,
                                        path);
                        if (!bb_ask_confirmation())
                                return 0;

What, syslogd will start talking to user? Daemonized syslogd???
[Well, not really, we try hard to prevent that by closing stdio,
but still... can we just unlink the pidfile?]

> The patch does not address udhcpc/udhcpd which still have a distinct
> pidfile writing code.

I recently touched it a bit, while trying to improve chances that
udhcp will work on NOMMU.

> Could this be merged in the trunk?

I would prefer another iteration. Start from attached patch...
--
vda

diff -d -urpN busybox.4/Config.in busybox.5/Config.in
--- busybox.4/Config.in	2007-03-25 21:32:08.000000000 +0200
+++ busybox.5/Config.in	2007-03-27 00:43:35.000000000 +0200
@@ -141,6 +141,13 @@ config FEATURE_CLEAN_UP
 	  Don't enable this unless you have a really good reason to clean
 	  things up manually.
 
+config FEATURE_PIDFILE
+	bool "Support writing pidfiles"
+	default n
+	help
+	  This option makes some applets (crond, syslogd and inetd) write
+	  a pidfile in /var/run. Some applications rely on them
+
 config FEATURE_SUID
 	bool "Support for SUID/SGID handling"
 	default n
diff -d -urpN busybox.4/include/libbb.h busybox.5/include/libbb.h
--- busybox.4/include/libbb.h	2007-03-26 20:31:47.000000000 +0200
+++ busybox.5/include/libbb.h	2007-03-27 00:43:35.000000000 +0200
@@ -542,6 +542,14 @@ extern void llist_unlink(llist_t **head,
 extern void llist_free(llist_t *elm, void (*freeit)(void *data));
 extern llist_t* llist_rev(llist_t *list);
 
+#if ENABLE_FEATURE_PIDFILE
+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
+
 enum {
 	LOGMODE_NONE = 0,
 	LOGMODE_STDIO = 1<<0,
diff -d -urpN busybox.4/libbb/Kbuild busybox.5/libbb/Kbuild
--- busybox.4/libbb/Kbuild	2007-03-25 21:32:01.000000000 +0200
+++ busybox.5/libbb/Kbuild	2007-03-27 00:43:35.000000000 +0200
@@ -61,6 +61,7 @@ lib-y += perror_msg.o
 lib-y += perror_msg_and_die.o
 lib-y += perror_nomsg.o
 lib-y += perror_nomsg_and_die.o
+lib-y += pidfile.o
 lib-y += process_escape_sequence.o
 lib-y += procps.o
 lib-y += read.o
diff -d -urpN busybox.4/libbb/pidfile.c busybox.5/libbb/pidfile.c
--- busybox.4/libbb/pidfile.c	1970-01-01 01:00:00.000000000 +0100
+++ busybox.5/libbb/pidfile.c	2007-03-27 00:04:04.000000000 +0200
@@ -0,0 +1,24 @@
+/* vi: set sw=4 ts=4: */
+/*
+ * pid file routines
+ *
+ * Copyright (C) 2006 by Stephane Billiart <[EMAIL PROTECTED]>
+ *
+ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
+ */
+#include "libbb.h"
+
+#if ENABLE_FEATURE_PIDFILE
+int writepidfile(const char *path)
+{
+	FILE *pidf;
+
+	pidf = fopen_or_warn(path, "w");
+	if (pidf) {
+		fprintf(pidf, "%u\n", getpid());
+		fclose(pidf);
+		return 0;
+	}
+	return 1;
+}
+#endif
diff -d -urpN busybox.4/miscutils/crond.c busybox.5/miscutils/crond.c
--- busybox.4/miscutils/crond.c	2007-03-26 20:31:49.000000000 +0200
+++ busybox.5/miscutils/crond.c	2007-03-27 00:43:35.000000000 +0200
@@ -185,6 +185,7 @@ int crond_main(int ac, char **av)
 		int rescan = 60;
 		short sleep_time = 60;
 
+		writepidfile("/var/run/crond.pid");
 		for (;;) {
 			sleep((sleep_time + 1) - (short) (time(NULL) % sleep_time));
 
diff -d -urpN busybox.4/networking/inetd.c busybox.5/networking/inetd.c
--- busybox.4/networking/inetd.c	2007-03-26 20:31:47.000000000 +0200
+++ busybox.5/networking/inetd.c	2007-03-27 00:43:35.000000000 +0200
@@ -1212,7 +1212,7 @@ static void goaway(int sig ATTRIBUTE_UNU
 		}
 		(void) close(sep->se_fd);
 	}
-	(void) unlink(_PATH_INETDPID);
+	removepidfile(_PATH_INETDPID);
 	exit(0);
 }
 
@@ -1301,13 +1301,7 @@ int inetd_main(int argc, char *argv[])
 		setgroups(1, &gid);
 	}
 
-	{
-		FILE *fp = fopen(_PATH_INETDPID, "w");
-		if (fp != NULL) {
-			fprintf(fp, "%u\n", getpid());
-			fclose(fp);
-		}
-	}
+	writepidfile(_PATH_INETDPID);
 
 	if (getrlimit(RLIMIT_NOFILE, &rlim_ofile) < 0) {
 		bb_perror_msg("getrlimit");
diff -d -urpN busybox.4/sysklogd/syslogd.c busybox.5/sysklogd/syslogd.c
--- busybox.4/sysklogd/syslogd.c	2007-03-26 20:31:48.000000000 +0200
+++ busybox.5/sysklogd/syslogd.c	2007-03-27 00:43:35.000000000 +0200
@@ -519,6 +519,7 @@ static void do_syslogd(void)
 	signal(SIGALRM, do_mark);
 	alarm(G.markInterval);
 #endif
+	removepidfile("/var/run/syslogd.pid");
 
 	memset(&sunx, 0, sizeof(sunx));
 	sunx.sun_family = AF_UNIX;
@@ -645,6 +646,7 @@ int syslogd_main(int argc, char **argv)
 		bb_daemonize_or_rexec(DAEMON_CHDIR_ROOT, argv);
 	}
 	umask(0);
+	writepidfile("/var/run/syslogd.pid");
 	do_syslogd();
 	/* return EXIT_SUCCESS; */
 }
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Reply via email to