[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 2/2] start-stop-daemon - find processes better
From:       Roy Marples <roy () marples ! name>
Date:       2008-04-30 16:19:16
Message-ID: 200804301719.16679.roy () marples ! name
[Download RAW message or body]

On Wednesday 30 April 2008 16:25:10 Joakim Tjernlund wrote:
> > But the intent of start-stop-daemon is to start and stop daemons.
> > s-s-d --stop --exec /usr/sbin/ntpd
> > I would that expect it to stop the currently running ntpd process
> > regardless of if the binary itself still exists or not.
>
> Don't think so, this is what --name is for. At least I think so.

--name is for checking the process name, or argv[0], which is different from 
how the daemon is started.

>
> > > > It
> > > > wont work when cmdline i a symlink. Should you not use --name instead
> > > > to handle the deleted case?
> >
> > But this is good :)
> > Consider busybox - loads of things symlink into it, like udhcpd. So are
> > you stopping udhcpd or all instances of busybox daemons?
>
> You won't stop anything with your solution because /proc/xxx/exe
> contains the real file name. So
>   s-s-d --stop --exec /bin/mysymlinkeddaemon
> will not match.

Good point.
New patch attached which just checks cmdline instead of doing exe then 
cmdline.

function                                             old     new   delta
start_stop_daemon_main                               984    1006     +22
.rodata                                           119312  119303      -9
check                                                755     714     -41
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/2 up/down: 22/-50)            Total: -28 bytes

So that's now an overall code shrink :)

Also you should consider the case of scripts. If /usr/bin/foo is  a python 
script, and was started like
start-stop-daemon --start --exec /usr/bin/foo
then you could reasonably expect
start-stop-daemon --stop --exec /usr/bin/foo
to work. However it won't because the exe inode points at python and not the 
script.

Thanks

Roy

["bb-ssd-proc.patch" (text/x-diff)]

diff -ur a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
--- a/debianutils/start_stop_daemon.c	2008-04-30 16:49:36.000000000 +0100
+++ b/debianutils/start_stop_daemon.c	2008-04-30 16:54:30.000000000 +0100
@@ -51,7 +51,6 @@
 	char *pidfile;
 	int user_id;
 	smallint signal_nr;
-	struct stat execstat;
 };
 #define G (*(struct globals*)&bb_common_bufsiz1)
 #define found             (G.found               )
@@ -61,7 +60,6 @@
 #define pidfile           (G.pidfile             )
 #define user_id           (G.user_id             )
 #define signal_nr         (G.signal_nr           )
-#define execstat          (G.execstat            )
 #define INIT_G() \
         do { \
 		user_id = -1; \
@@ -71,15 +69,17 @@
 
 static int pid_is_exec(pid_t pid)
 {
-	struct stat st;
-	char buf[sizeof("/proc//exe") + sizeof(int)*3];
-
-	sprintf(buf, "/proc/%u/exe", pid);
-	if (stat(buf, &st) < 0)
-		return 0;
-	if (st.st_dev == execstat.st_dev
-	 && st.st_ino == execstat.st_ino)
-		return 1;
+	char cmdline[32];
+	ssize_t bytes;
+	char buf[PATH_MAX];
+
+	snprintf(cmdline, sizeof(cmdline), "/proc/%u/cmdline", pid);
+	bytes = open_read_close(cmdline, buf, sizeof(buf));
+	if (bytes > 0) {
+		buf[bytes] = '\0';
+		if (strcmp(buf, execname) == 0)
+			return 1;
+	}
 	return 0;
 }
 
@@ -257,6 +257,7 @@
 	char *signame;
 	char *startas;
 	char *chuid;
+	struct stat execstat;
 #if ENABLE_FEATURE_START_STOP_DAEMON_FANCY
 //	char *retry_arg = NULL;
 //	int retries = -1;
@@ -304,9 +305,6 @@
 		if (errno)
 			user_id = xuname2uid(userspec);
 	}
-	if (execname)
-		xstat(execname, &execstat);
-
 	do_procinit(); /* Both start and stop needs to know current processes */
 
 	if (opt & CTX_STOP) {
@@ -319,6 +317,10 @@
 			printf("%s already running\n%d\n", execname, found->pid);
 		return !(opt & OPT_OKNODO);
 	}
+
+	if (execname)
+		xstat(execname, &execstat);
+
 	*--argv = startas;
 	if (opt & OPT_BACKGROUND) {
 #if BB_MMU


_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread]