Merge remote-tracking branch 'refs/remotes/origin/master'

* refs/remotes/origin/master:
  locking.c, locking.1: Make the protocol safe if the lockfile is (re)moved.
  locking.c: Refactor the main locking flow.
  locking.c: Use shell convention to report child killed by a signal.
  locking.c: Rename exit status variable to `rc'.
  locking.c: Compress vertically.  Remove pointless `$Id' comment.

diff --git a/locking.1 b/locking.1
index 51837902af5c222e763919baf6687853b8be363a..a9a37aa52fee905b7df2e02803955744b81f8e35 100644 (file)
--- a/locking.1
+++ b/locking.1
@@ -97,6 +97,21 @@ for days, hours, minutes or seconds, respectively) for the lock to
 become available, and then give up.  This only makes sense with the
 .B \-\-wait
 option, so that is turned on automatically.
+.PP
+It is safe to unlink or atomically replace the lockfile while holding
+the lock, though these actions will release the lock immediately.  To
+safely delete the lockfile, for example, run
+.IP
+.B "locking lock rm lock"
+.PP
+Similarly, a file can be updated safely by
+.IP
+.nf
+.ft B
+locking file sh -c \e
+\h'8m'"update-file file >file.new && mv file.new file"
+.fi
+.ft R
 .SH "BUGS"
 The
 .B locking

diff --git a/locking.c b/locking.c
index 8931dbd7dc5d382e0514452232eac703b8c177e2..bfc5c771927e13d04703108224992ce92aef6933 100644 (file)
--- a/locking.c
+++ b/locking.c
@@ -1,6 +1,4 @@
 /* -*-c-*-
- *
- * $Id$
  *
  * Lock a file, run a program
  *
@@ -41,6 +39,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/wait.h>
+#include <sys/stat.h>
 
 #include <mLib/mdwopt.h>
 #include <mLib/quis.h>
@@ -49,6 +48,7 @@
 /*----- Static variables --------------------------------------------------*/
 
 static jmp_buf jmp;
+static int err;
 
 /*----- Main code ---------------------------------------------------------*/
 
@@ -61,9 +61,7 @@ static void usage(FILE *fp)
 }
 
 static void version(FILE *fp)
-{
-  pquis(fp, "$ (version " VERSION ")\n");
-}
+  { pquis(fp, "$ (version " VERSION ")\n"); }
 
 static void help(FILE *fp)
 {
@@ -92,14 +90,16 @@ int main(int argc, char *argv[])
   const char *prog = 0;
   char *const *av;
   void (*oalrm)(int) = 0;
-  int fd;
+  int fd = -1;
   struct flock l;
   char *p;
   int t = -1;
+  int oflag;
   unsigned int ot = 0;
+  struct stat st, nst;
   time_t nt;
   pid_t kid;
-  int st;
+  int rc;
 
 #define f_bogus 1u
 #define f_wait 2u
@@ -127,42 +127,19 @@ int main(int argc, char *argv[])
 
     int i = mdwopt(argc, argv, "-hvuw+f+c+x+p:t:", opts,
                   0, 0, OPTF_NEGATION);
-    if (i < 0)
-      break;
+    if (i < 0) break;
     switch (i) {
-      case 'h':
-       help(stdout);
-       exit(0);
-      case 'v':
-       version(stdout);
-       exit(0);
-      case 'u':
-       usage(stdout);
-       exit(0);
-      case 'w':
-       f |= f_wait;
-       break;
-      case 'w' | OPTF_NEGATED:
-       f &= ~f_wait;
-       break;
-      case 'f':
-       f |= f_fail;
-       break;
-      case 'f' | OPTF_NEGATED:
-       f &= ~f_fail;
-       break;
-      case 'c':
-       f |= f_create;
-       break;
-      case 'c' | OPTF_NEGATED:
-       f &= ~f_create;
-       break;
-      case 'x':
-       f |= f_excl;
-       break;
-      case 'x' | OPTF_NEGATED:
-       f &= ~f_excl;
-       break;
+      case 'h': help(stdout); exit(0);
+      case 'v': version(stdout); exit(0);
+      case 'u': usage(stdout); exit(0);
+      case 'w': f |= f_wait; break;
+      case 'w' | OPTF_NEGATED: f &= ~f_wait; break;
+      case 'f': f |= f_fail; break;
+      case 'f' | OPTF_NEGATED: f &= ~f_fail; break;
+      case 'c': f |= f_create; break;
+      case 'c' | OPTF_NEGATED: f &= ~f_create; break;
+      case 'x': f |= f_excl; break;
+      case 'x' | OPTF_NEGATED: f &= ~f_excl; break;
       case 't':
        errno = 0;
        t = strtol(optarg, &p, 0);
@@ -174,23 +151,15 @@ int main(int argc, char *argv[])
          case 0: break;
          default: die(111, "unknown time unit `%c'", *p);
        }
-       if (*p || t < 0 || errno)
-         die(111, "bad time value `%s'", optarg);
+       if (*p || t < 0 || errno) die(111, "bad time value `%s'", optarg);
        f |= f_wait;
        break;
-      case 'p':
-       prog = optarg;
-       break;
+      case 'p': prog = optarg; break;
       case 0:
-       if (file) {
-         optind--;
-         goto doneopts;
-       }
+       if (file) { optind--; goto doneopts; }
        file = optarg;
        break;
-      default:
-       f |= f_bogus;
-       break;
+      default: f |= f_bogus; break;
     }
   }
 
@@ -201,64 +170,70 @@ doneopts:
   }
 
   av = &argv[optind];
-  if (!prog)
-    prog = av[0];
-  if ((fd = open(file,
-                ((f & f_create ? O_CREAT : 0) |
-                 (f & f_excl ? O_RDWR : O_RDONLY)), 0666)) < 0)
-    die(111, "error opening `%s': %s", file, strerror(errno));
+  if (!prog) prog = av[0];
+  oflag = f & f_excl ? O_RDWR : O_RDONLY;
+  if (f & f_create) oflag |= O_CREAT;
   l.l_type = f & f_excl ? F_WRLCK : F_RDLCK;
   l.l_whence = SEEK_SET;
   l.l_start = 0;
   l.l_len = 0;
   nt = time(0);
   if (setjmp(jmp)) {
-    errno = EAGAIN;
+    err = EAGAIN;
     nt = t;
-  } else {
-    ot = alarm(0);
-    oalrm = signal(SIGALRM, alrm);
-    if (t >= 0)
-      alarm(t);
-    if (fcntl(fd, f & f_wait ? F_SETLKW : F_SETLK, &l) >= 0)
-      errno = 0;
+    goto done;
+  }
+  ot = alarm(0);
+  oalrm = signal(SIGALRM, alrm);
+  if (t >= 0) alarm(t);
+again:
+  if ((fd = open(file, oflag, 0666)) < 0)
+    die(111, "error opening `%s': %s", file, strerror(errno));
+  if (fstat(fd, &st))
+    die(111, "error from fstat on `%s': %s", file, strerror(errno));
+  err = fcntl(fd, f & f_wait ? F_SETLKW : F_SETLK, &l) >= 0 ? 0 : errno;
+  if (stat(file, &nst)) {
+    if (errno == ENOENT) { close(fd); goto again; }
+    else die(111, "error from stat on `%s': %s", file, strerror(errno));
   }
+  if (st.st_dev != nst.st_dev || st.st_ino != nst.st_ino)
+    { close(fd); goto again; }
+done:
   signal(SIGALRM, oalrm);
   if (!ot)
     alarm(0);
   else {
     nt = time(0) - nt;
-    if (nt > ot)
-      raise(SIGALRM);
-    else
-      alarm(ot - nt);
+    if (nt > ot) raise(SIGALRM);
+    else alarm(ot - nt);
   }
-  if (errno &&
-      ((errno != EAGAIN && errno != EWOULDBLOCK && errno != EACCES) ||
-       (f & f_fail)))
-    die(111, "error locking `%s': %s", file, strerror(errno));
-  if (errno)
-    exit(0);
-
-  if ((kid = fork()) < 0)
-    die(111, "error from fork: %s", strerror(errno));
+  switch (err) {
+    case 0: break;
+    case EAGAIN:
+#if defined(EWOULDBLOCK) && EWOULDBLOCK != EAGAIN
+    case EWOULDBLOCK:
+#endif
+    case EACCES:
+      if (!(f & f_fail)) exit(0);
+    default:
+      die(111, "error locking `%s': %s", file, strerror(errno));
+  }
+  if ((kid = fork()) < 0) die(111, "error from fork: %s", strerror(errno));
   if (!kid) {
     close(fd);
     execvp(prog, av);
     die(111, "couldn't exec `%s': %s", prog, strerror(errno));
   }
-  if (waitpid(kid, &st, 0) < 0)
+  if (waitpid(kid, &rc, 0) < 0)
     die(EXIT_FAILURE, "error from wait: %s", strerror(errno));
   l.l_type = F_UNLCK;
   l.l_whence = SEEK_SET;
   l.l_start = 0;
   l.l_len = 0;
   fcntl(fd, F_SETLK, &l);
-  close(fd);
-  if (WIFEXITED(st))
-    exit(WEXITSTATUS(st));
-  else
-    exit(255);
+  if (fd >= 0) close(fd);
+  if (WIFEXITED(rc)) exit(WEXITSTATUS(rc));
+  else exit(128 + WTERMSIG(rc));
 }
 
 /*----- That's all, folks -------------------------------------------------*/