On Wed, 2006-02-15 at 15:14 +0100, Javier Fernández-Sanguino Peña wrote:

> Hi there. I hope you don't mind me being a little bit picky, but I think it
> helps you hone your skills :-)

I've certainly picked up a couple of pointers. 

> I'm pretty sure there are libraries that do implement that function already.
> For exmaple, the 'daemon' package in Debian, which is based on libslack (see
> http://libslack.org/) already provides a number of library functions for
> daemons, such as daemon_pidfile (see
> http://libslack.org/manpages/daemon.3.html). Most of the code you want
> is already there (see daemon_construct_pidfile() and
> daemon_lock_pidfile() or daemon_pidfile_unlocked)

A useful little library indeed, thanks for the pointer. It is very nice
and elegant code. It's still not suitable for wholesale inclusion as it
has its own memory management routines that are used in
daemon_construct_pidfile and daemon_pidfile_unlocked, but I've managed
to reuse the daemon_lock_pidfile function with a couple of small
wrappers to do the job.  

The patch is now back down to the size/scope that I consider appropriate
for a NMU, I agree that the previous patch was getting a little unwieldy
and rough, my apologies. 

> Would you mind looking up how daemon() does this and maybe implement a
> similar solution?

Please see the attached patch. 

Cheers

-- 
Matt Brown
[EMAIL PROTECTED]
Mob +64 21 611 544 www.mattb.net.nz
diff -ur portreserve-0.0.0/debian/changelog portreserve-0.0.0-matt/debian/changelog
--- portreserve-0.0.0/debian/changelog	2006-02-12 11:07:31.000000000 +0000
+++ portreserve-0.0.0-matt/debian/changelog	2006-02-16 08:42:01.000000000 +0000
@@ -1,3 +1,15 @@
+portreserve (0.0.0-2.1) unstable; urgency=low
+
+  * Non-maintainer upload
+  * Fixed minor init script bugs (Closes: #352103)
+    - Use -z instead of -n to test list of service files
+    - Use $NAME instead of the undefined $prog in the pidfile name
+  * Reworked portreserve pidfile handling
+    - Utilise functions from libslack to safely create and lock pidfile
+    - Remove pidfile when program exits cleanly
+  
+ -- Matt Brown <[EMAIL PROTECTED]>  Thu, 16 Feb 2006 21:41:33 +1300
+
 portreserve (0.0.0-2) unstable; urgency=low
 
   * Added xmlto to Build-Depends (Closes: #337848)
diff -ur portreserve-0.0.0/debian/portreserve.init portreserve-0.0.0-matt/debian/portreserve.init
--- portreserve-0.0.0/debian/portreserve.init	2006-02-12 11:07:31.000000000 +0000
+++ portreserve-0.0.0-matt/debian/portreserve.init	2006-02-15 11:43:34.000000000 +0000
@@ -11,7 +11,7 @@
 test -f $DAEMON || exit 0
 
 NAME=`basename $DAEMON`
-PIDFILE=/var/run/$prog.pid
+PIDFILE=/var/run/$NAME.pid
 
 running()
 {
@@ -22,8 +22,8 @@
     # No pid, probably no daemon present
     [ -z "$pid" ] && return 1
     [ ! -d /proc/$pid ] &&  return 1
-    cmd=`cat /proc/$pid/cmdline | tr "\000" "\n"|head -n 1 |cut -d : -f 1`
-
+    cmdline=`cat /proc/$pid/cmdline | tr "\000" "\n"|head -n 1 |cut -d : -f 1`
+    cmd=`basename $cmdline`
     [ "$cmd" != "$NAME" ] &&  return 1
     return 0
 }
@@ -36,7 +36,7 @@
 	if [ ! -d /etc/portreserve ] ; then
 		return 1
 	fi
-	if [ -n "`find /etc/portreserve \! -name "*~" -a \! -name "*.*" -type f`" ] ; then
+	if [ -z "`find /etc/portreserve \! -name "*~" -a \! -name "*.*" -type f`" ] ; then
 		return 1
 	fi
 	return 0
Only in portreserve-0.0.0-matt/: .deps
Only in portreserve-0.0.0: portreserve.spec
diff -ur portreserve-0.0.0/src/portreserve.c portreserve-0.0.0-matt/src/portreserve.c
--- portreserve-0.0.0/src/portreserve.c	2003-09-03 14:12:52.000000000 +0000
+++ portreserve-0.0.0-matt/src/portreserve.c	2006-02-16 08:34:09.000000000 +0000
@@ -64,7 +64,13 @@
 # include <unistd.h>
 #endif
 
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
 #define UNIX_SOCKET "/var/run/portreserve/socket"
+#define PIDFILE "/var/run/portreserve.pid"
 
 struct map {
 	struct map *next;
@@ -264,9 +270,85 @@
 	return 0;
 }
 
+/* daemon_lock_pidfile and fcntl_lock taken from libslack 
+ * Copyright (C) 1999-2004 raf <[EMAIL PROTECTED]>
+ * Licensed under the GPL
+ */
+int 
+fcntl_lock(int fd, int cmd, int type, int whence, int start, int len)
+{
+        struct flock lock[1];
+
+        lock->l_type = type;
+        lock->l_whence = whence;
+        lock->l_start = start;
+        lock->l_len = len;
+
+        return fcntl(fd, cmd, lock);
+}
+
+static int 
+daemon_lock_pidfile(char *pidfile)
+{
+        mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+        int pid_fd;
+
+        /* This is broken over NFS (Linux). So pidfiles must reside locally. */
+
+        if ((pid_fd = open(pidfile, O_RDWR | O_CREAT | O_EXCL, mode)) == -1)
+        {
+                if (errno != EEXIST)
+                        return -1;
+
+                /*
+                ** The pidfile already exists. Is it locked?
+                ** If so, another invocation is still alive.
+                ** If not, the invocation that created it has died.
+                ** Open the pidfile to attempt a lock.
+                */
+
+                if ((pid_fd = open(pidfile, O_RDWR)) == -1)
+                        return -1;
+        }
+
+        if (fcntl_lock(pid_fd, F_SETLK, F_WRLCK, SEEK_SET, 0, 0) == -1)
+                return -1;
+
+        return pid_fd;
+}
+
+int 
+create_pidfile(char *pidfile)
+{
+
+	int fd;
+	char pid[32];
+	
+	/* Open and lock pidfile */
+	if ((fd = daemon_lock_pidfile(pidfile)) == -1) {
+		return -1;
+	}
+
+	/* Store our pid */
+	snprintf(pid, sizeof(pid), "%d\n", (int)getpid());
+	if (write(fd, pid, strlen(pid)) != strlen(pid)) {
+		return -1;
+	}
+
+	return 0;
+}
+
+void
+handle_sigterm (int sig)
+{
+	unlink(PIDFILE);
+	exit(0);
+}
+
 int
 main (int argc, char **argv)
 {
+	int rv;
 	const char *p = strrchr (argv[0], '/');
 	if (!p++)
 		p = argv[0];
@@ -278,7 +360,9 @@
 			r += portrelease (argv[i]);
 		return r;
 	}
-
+	
+	signal (SIGTERM, handle_sigterm);
+	
 	if (argc > 1) {
 		if (!strcmp (argv[1], "-d"))
 			debug = 1;
@@ -302,7 +386,12 @@
 		close (STDOUT_FILENO);
 		close (STDERR_FILENO);
 		setsid ();
+		if (create_pidfile(PIDFILE)==-1)
+			error (EXIT_FAILURE, errno, 
+					"Failed to write pidfile!");
 	}
 
-	return portreserve ();
+	rv = portreserve();
+	unlink(PIDFILE);
+	return rv;
 }

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to