some improvements and fixes

Signed-off-by: Mattias Andrée <maandree@member.fsf.org>

4 files changed, 113 insertions, 78 deletions

diff --git a/src/client.c b/src/client.c
index 1bf8440..a0f58e3 100644
--- a/src/client.c
+++ b/src/client.c
@@ -63,17 +63,17 @@ send_command(enum command cmd, size_t n, const char *restrict msg)
 
 	/* Get socket address. */
 	dir = getenv("XDG_RUNTIME_DIR"), dir = (dir ? dir : "/run");
-	if (strlen(dir) + sizeof("/satd.socket") > sizeof(address.sun_path))
+	if (strlen(dir) + sizeof("/" PACKAGE "/socket") > sizeof(address.sun_path))
 		t ((errno = ENAMETOOLONG));
-	stpcpy(stpcpy(address.sun_path, dir), "/satd.socket");
+	stpcpy(stpcpy(address.sun_path, dir), "/" PACKAGE "/state"); /* Yes, "state", It is temporary. */
 	address.sun_family = AF_UNIX;
 
 	/* Any daemon listening? */
 	fd = open(address.sun_path, O_RDONLY);
 	if (fd == -1) {
-		t (errno != ENOENT);
+		t ((errno != ENOENT) && (errno != ENOTDIR));
 	} else {
-		if (flock(fd, LOCK_SH | LOCK_NB) == -1)
+		if (flock(fd, LOCK_EX | LOCK_NB /* and LOCK_DRY if that was ever added... */))
 			t (start = 0, errno != EWOULDBLOCK);
 		else
 			flock(fd, LOCK_UN);
@@ -97,6 +97,7 @@ send_command(enum command cmd, size_t n, const char *restrict msg)
 	}
 
 	/* Create socket. */
+	stpcpy(strrchr(address.sun_path, '/'), "/socket");
 	t ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) == -1);
 	t (connect(fd, (const struct sockaddr *)(_cvoid = &address), (socklen_t)sizeof(address)) == -1);
 
diff --git a/src/daemonise.c b/src/daemonise.c
index c9de08a..f8da122 100644
--- a/src/daemonise.c
+++ b/src/daemonise.c
@@ -60,7 +60,7 @@ static char* __pidfile = NULL;
  */
 static int dup_at_least_3(int old)
 {
-  int intermediary[] = { -1, -1, -1 };
+  int intermediary[3];
   int i = 0, saved_errno;
   
   do
@@ -73,12 +73,12 @@ static int dup_at_least_3(int old)
       intermediary[i++] = old;
     }
   while (old < 3);
+  i--;
   
  fail:
   saved_errno = errno;
-  if (intermediary[0] >= 0)  close(intermediary[0]);
-  if (intermediary[1] >= 0)  close(intermediary[1]);
-  if (intermediary[2] >= 0)  close(intermediary[2]);
+  while (i--)
+    close(intermediary[i]);
   errno = saved_errno;
   return old;
 }
@@ -167,6 +167,7 @@ static int dup_at_least_3(int old)
  *                 -  `DAEMONISE_KEEP_STDIN`
  *                 -  `DAEMONISE_KEEP_STDOUT`
  *                 -  `DAEMONISE_KEEP_FDS`
+ *                 -  `DAEMONISE_NEW_PID`
  * @param   ...    Enabled if `DAEMONISE_KEEP_FDS` is used,
  *                 do not add anything if `DAEMONISE_KEEP_FDS`
  *                 is unused. This is a `-1`-terminated list
@@ -182,9 +183,10 @@ static int dup_at_least_3(int old)
  *                  has exited without removing the PID file.
  * @throws  EINVAL  `flags` contains an unsupported bit, both
  *                  `DAEMONISE_KEEP_STDERR` and `DAEMONISE_CLOSE_STDERR`
- *                  are set, or both `DAEMONISE_CLOSE_STDERR` and
- *                  `DAEMONISE_KEEP_FDS` are set whilst `2` is
- *                  in the list of file descriptor not to close,
+ *                  are set, both `DAEMONISE_NO_PID_FILE` and
+ *                  `DAEMONISE_NEW_PID`, or both `DAEMONISE_CLOSE_STDERR`
+ *                  and `DAEMONISE_KEEP_FDS` are set whilst `2` is
+ *                  in the list of file descriptor not to close.
  * @throws          Any error specified for signal(3).
  * @throws          Any error specified for sigemptyset(3).
  * @throws          Any error specified for sigprocmask(3).
@@ -218,20 +220,22 @@ int daemonise(const char* name, int flags, ...)
   
   
   /* Validate flags. */
-  if (flags & ~2047)
+  if (flags & (int)~(2048L * 2 - 1))
     return errno = EINVAL, -1;
-  if (flags & DAEMONISE_KEEP_STDERR)
-    if (flags & DAEMONISE_CLOSE_STDERR)
+  if ((flags & DAEMONISE_KEEP_STDERR) && (flags & DAEMONISE_CLOSE_STDERR))
+      return errno = EINVAL, -1;
+  if ((flags & DAEMONISE_NO_PID_FILE) && (flags & DAEMONISE_NEW_PID))
       return errno = EINVAL, -1;
   
   
   /* Find out which file descriptors not too close. */
-  if ((flags & DAEMONISE_KEEP_FDS) == 0)
+  if (flags & DAEMONISE_KEEP_FDS)
     {
       va_start(args, flags);
-      while (va_arg(args, int) >= 0)
+      while ((fd = va_arg(args, int)) >= 0)
 	if ((fd > 2) && (keepmax < fd))
 	  keepmax = fd;
+      fd = -1;
       va_end(args);
       keep = calloc((size_t)keepmax + 1, sizeof(char));
       t (keep == NULL);
@@ -249,8 +253,8 @@ int daemonise(const char* name, int flags, ...)
 	    keep[fd] = 1;
 	    break;
 	  }
-      va_end(args);
       fd = -1;
+      va_end(args);
     }
   /* We assume that the maximum file descriptor is not extremely large.
    * We also assume the number of file descriptors too keep is very small,
@@ -264,7 +268,7 @@ int daemonise(const char* name, int flags, ...)
       for (i = 3; (rlim_t)i < rlimit.rlim_cur; i++)
 	/* File descriptors with numbers above and including
 	 * `rlimit.rlim_cur` cannot be created. They cause EBADF. */
-	if ((keep == NULL) || (keep[i] == 0))
+	if ((i > keepmax) || (keep[i] == 0))
 	  close(i);
     }
   free(keep), keep = NULL;
@@ -272,7 +276,8 @@ int daemonise(const char* name, int flags, ...)
   /* Reset all signal handlers. */
   if ((flags & DAEMONISE_NO_SIG_DFL) == 0)
     for (i = 1; i < _NSIG; i++)
-      t (signal(i, SIG_DFL) == SIG_ERR);
+      if (signal(i, SIG_DFL) == SIG_ERR)
+	t (errno != EINVAL);
   
   /* Set signal mask. */
   if ((flags & DAEMONISE_KEEP_SIGMASK) == 0)
@@ -337,7 +342,7 @@ int daemonise(const char* name, int flags, ...)
       t (__pidfile == NULL);
       stpcpy(stpcpy(stpcpy(__pidfile, "/run/"), name), ".pid");
     }
-  fd = open(__pidfile, O_WRONLY | O_CREAT | O_EXCL, 0644);
+  fd = open(__pidfile, O_WRONLY | O_CREAT | ((flags & DAEMONISE_NEW_PID) ? 0 : O_EXCL), 0644);
   if (fd == -1)
     {
       saved_errno = errno;
diff --git a/src/daemonise.h b/src/daemonise.h
index c590174..6585154 100644
--- a/src/daemonise.h
+++ b/src/daemonise.h
@@ -50,6 +50,8 @@
 
 /**
  * Do not create a PID file.
+ * 
+ * Cannot be combined with `DAEMONISE_NEW_PID`.
  */
 #define DAEMONISE_NO_PID_FILE  32
 
@@ -57,7 +59,7 @@
  * Do not close stderr even if it is
  * a terminal device.
  * 
- * Cannot be combined with `DAEMONISE_KEEP_STDERR`.
+ * Cannot be combined with `DAEMONISE_CLOSE_STDERR`.
  */
 #define DAEMONISE_KEEP_STDERR  64
 
@@ -85,6 +87,16 @@
  */
 #define DAEMONISE_KEEP_FDS  1024
 
+/**
+ * Override the PID file if it already exists,
+ * rather than failing. It is a bad idea to do
+ * this unless you already made sure that the
+ * daemon is not already running.
+ * 
+ * Cannot be combined with `DAEMONISE_NO_PID_FILE`.
+ */
+#define DAEMONISE_NEW_PID  2048
+
 
 
 /**
@@ -167,6 +179,7 @@
  *                 -  `DAEMONISE_KEEP_STDIN`
  *                 -  `DAEMONISE_KEEP_STDOUT`
  *                 -  `DAEMONISE_KEEP_FDS`
+ *                 -  `DAEMONISE_NEW_PID`
  * @param   ...    Enabled if `DAEMONISE_KEEP_FDS` is used,
  *                 do not add anything if `DAEMONISE_KEEP_FDS`
  *                 is unused. This is a `-1`-terminated list
@@ -182,8 +195,9 @@
  *                  has exited without removing the PID file.
  * @throws  EINVAL  `flags` contains an unsupported bit, both
  *                  `DAEMONISE_KEEP_STDERR` and `DAEMONISE_CLOSE_STDERR`
- *                  are set, or both `DAEMONISE_CLOSE_STDERR` and
- *                  `DAEMONISE_KEEP_FDS` are set whilst `2` is
+ *                  are set, both `DAEMONISE_NO_PID_FILE` and
+ *                  `DAEMONISE_NEW_PID`, or both `DAEMONISE_CLOSE_STDERR`
+ *                  and `DAEMONISE_KEEP_FDS` are set whilst `2` is
  *                  in the list of file descriptor not to close.
  * @throws          Any error specified for signal(3).
  * @throws          Any error specified for sigemptyset(3).
diff --git a/src/satd.c b/src/satd.c
index df2cda9..0b8160c 100644
--- a/src/satd.c
+++ b/src/satd.c
@@ -48,7 +48,7 @@ USAGE("[-f]")
 
 
 /**
- * Create the socket.
+ * Create the socket, but not its directory.
  * 
  * @param   address  Output parameter for the socket address.
  * @return           The file descriptor for the socket, -1 on error.
@@ -63,37 +63,19 @@ create_socket(struct sockaddr_un *address)
 
 	/* Get socket address. */
 	dir = getenv("XDG_RUNTIME_DIR"), dir = (dir ? dir : "/run");
-	if (strlen(dir) + sizeof("/satd.socket") > sizeof(address->sun_path))
+	if (strlen(dir) + sizeof("/" PACKAGE "/socket") > sizeof(address->sun_path))
 		t ((errno = ENAMETOOLONG));
-	stpcpy(stpcpy(address->sun_path, dir), "/satd.socket");
+	stpcpy(stpcpy(address->sun_path, dir), "/" PACKAGE "/socket");
 	address->sun_family = AF_UNIX;
 
-	/* Check that no living process owns the socket. */
-	fd = open(address->sun_path, O_RDONLY);
-	if (fd == -1) {
-		t (errno != ENOENT);
-		goto does_not_exist;
-	} else if (flock(fd, LOCK_EX | LOCK_NB) == -1) {
-		t (errno != EWOULDBLOCK);
-		fprintf(stderr, "%s: the daemon's socket file is already in use.\n", argv0);
-		errno = 0;
-		goto fail;
-	}
-
 	/* Create socket. */
 	unlink(address->sun_path);
-	flock(fd, LOCK_UN);
-	close(fd), fd = -1;
-does_not_exist:
 	t ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) == -1);
 	t (fchmod(fd, S_IRWXU) == -1);
 	t (bind(fd, (const struct sockaddr *)(_cvoid = address), (socklen_t)sizeof(*address)) == -1);
 	/* EADDRINUSE just means that the file already exists, not that it is actually used. */
 	bound = 1;
 
-	/* Mark the socket as owned by a living process. */
-	t (flock(fd, LOCK_SH));
-
 	return fd;
 fail:
 	saved_errno = errno;
@@ -107,6 +89,51 @@ fail:
 
 
 /**
+ * Create the state file, and its directory.
+ * 
+ * This will also check that the daemon is not
+ * running. The daemon holds a shared lock on
+ * the state file.
+ * 
+ * @return  A file descriptor to the state file, -1 on error.
+ */
+static int
+create_state(void)
+{
+	const char *dir;
+	char *path;
+	char *p;
+	int fd, saved_errno;
+
+	/* Create directory. */
+	dir = getenv("XDG_RUNTIME_DIR"), dir = (dir ? dir : "/run");
+	t (!(path = malloc(strlen(dir) * sizeof(char) + sizeof("/" PACKAGE "/state"))));
+	p = stpcpy(stpcpy(path, dir), "/" PACKAGE);
+	t (mkdir(path, S_IRWXU) && (errno != EEXIST));
+
+	/* Open file. */
+	stpcpy(p, "/state");
+	t (fd = open(path, O_RDWR | O_CREAT /* but not O_EXCL */, S_IRUSR | S_IWUSR), fd == -1);
+	free(path), path = NULL;
+
+	/* Check that the daemon is not running, and mark it as running. */
+	if (flock(fd, LOCK_EX | LOCK_NB)) {
+		t (errno != EWOULDBLOCK);
+		fprintf(stderr, "%s: the daemon's state file is already in use.\n", argv0);
+		errno = 0;
+		goto fail;
+	}
+
+	return fd;
+fail:
+	saved_errno = errno;
+	free(path);
+	errno = saved_errno;
+	return -1;
+}
+
+
+/**
  * Construct the pathname for the hook script.
  * 
  * @param   env     The environment variable to use for the beginning
@@ -159,16 +186,16 @@ fail:
 static int
 dup2_and_null(int old, int new)
 {
-	int want, fd = -1;
+	int fd = -1;
 	int saved_errno;
 
 	if (old == new)  goto done;
 	t (dup2(old, new) == -1);
-	close(old), want = old;
-	if (want >= 3)   goto done;
+	close(old);
+	if (old >= 3)   goto done;
 	t (fd = open("/dev/null", O_RDWR), fd == -1);
-	if (fd == want)  goto done;
-	t (dup2(fd, want) == -1);
+	if (fd == old)  goto done;
+	t (dup2(fd, old) == -1);
 	close(fd), fd = -1;
 
 done:
@@ -195,11 +222,17 @@ fail:
 int
 main(int argc, char *argv[])
 {
+#define GET_FD(FD, WANT, CALL)              \
+	t (FD = CALL, FD == -1);            \
+	t (dup2_and_null(FD, WANT) == -1);  \
+	FD = WANT
+#define HOOKPATH(PRE, SUF)  \
+	t (path = path ? path : hookpath(PRE, SUF), !path && errno)
+
 	struct sockaddr_un address;
 	int sock = -1, state = -1, boot = -1, real = -1, foreground = 0;
 	char *path = NULL;
 	struct itimerspec spec;
-	const char *dir;
 
 	/* Parse command line. */
 	if (argc > 0)  argv0 = argv[0];
@@ -210,38 +243,20 @@ main(int argc, char *argv[])
 
 	/* Get hook-script pathname. */
 	if (!getenv("SAT_HOOK_PATH")) {
-		t (path =               hookpath("XDG_CONFIG_HOME", "/sat/hook"), !path && errno);
-		t (path = path ? path : hookpath("HOME", "/.config/sat/hook"),    !path && errno);
-		t (path = path ? path : hookpath(NULL, "/.config/sat/hook"),      !path && errno);
+		HOOKPATH("XDG_CONFIG_HOME", "/sat/hook");
+		HOOKPATH("HOME", "/.config/sat/hook");
+		HOOKPATH(NULL, "/.config/sat/hook");
 		t (setenv("SAT_HOOK_PATH", path ? path : "/etc/sat/hook", 1));
 		free(path), path = NULL;
 	}
 
-	/* Open/create state file. */
-	dir = getenv("XDG_RUNTIME_DIR"), dir = (dir ? dir : "/run");
-	t (!(path = malloc(strlen(dir) * sizeof(char) + sizeof("/satd.state"))));
-	stpcpy(stpcpy(path, dir), "/satd.state");
-	t (state = open(path, O_RDWR | O_CREAT /* but not O_EXCL */, S_IRWXU), state == -1);
-	free(path), path = NULL;
-
-	/* The state file shall be on fd STATE_FILENO. */
-	t (dup2_and_null(state, STATE_FILENO) == -1);
-	state = STATE_FILENO;
-
-	/* Create socket. */
-	t (sock = create_socket(&address), sock == -1);
-	t (dup2_and_null(sock, SOCK_FILENO) == -1);
-	sock = SOCK_FILENO;
-
-	/* Create CLOCK_BOOTTIME timer. */
-	t (boot = timerfd_create(CLOCK_BOOTTIME, 0), boot == -1);
-	t (dup2_and_null(boot, BOOT_FILENO) == -1);
-	boot = BOOT_FILENO;
+	/* Open/create state file, and create socket. */
+	GET_FD(state, STATE_FILENO, create_state());
+	GET_FD(sock,  SOCK_FILENO,  create_socket(&address));
 
-	/* Create CLOCK_REALTIME timer. */
-	t (real = timerfd_create(CLOCK_REALTIME, 0), real == -1);
-	t (dup2_and_null(real, BOOT_FILENO) == -1);
-	real = BOOT_FILENO;
+	/* Create timers. */
+	GET_FD(boot, BOOT_FILENO, timerfd_create(CLOCK_BOOTTIME, 0));
+	GET_FD(real, REAL_FILENO, timerfd_create(CLOCK_REALTIME, 0));
 
 	/* Configure timers. */
 	memset(&spec, 0, sizeof(spec));
@@ -256,7 +271,7 @@ main(int argc, char *argv[])
 #endif
 
 	/* Daemonise. */
-	t (foreground ? 0 : daemonise("satd", DAEMONISE_KEEP_FDS, 3, 4, 5, 6, -1));
+	t (foreground ? 0 : daemonise("satd", DAEMONISE_KEEP_FDS | DAEMONISE_NEW_PID, 3, 4, 5, 6, -1));
 
 	/* Change to a process image without all this initialisation text. */
 	execl(LIBEXECDIR "/" PACKAGE "/satd-diminished", argv0, address.sun_path, NULL);