From: Mark Wooding <mdw@distorted.org.uk>
Date: Sun, 24 Apr 2016 22:30:30 +0000 (+0100)
Subject: locking.c: Refactor the main locking flow.
X-Git-Tag: 1.4.2~1^2~1
X-Git-Url: https://git.distorted.org.uk/~mdw/misc/commitdiff_plain/b7ec4e9118fa390ec20408390a06f3c51a035155

locking.c: Refactor the main locking flow.

  * Calculate the open(2) flags ahead of time.

  * Move the open(2) until after we set up the alarm.  This means that
    we might not actually set `fd' at all, if we get delayed and the
    alarm goes off.

  * Use a separate static (because setjmp(3)) variable to record the
    fcntl(2) return code.

  * Use `goto' and labels for the control flow, rather than having
    everything in an `else' branch.

  * Use `switch' to pick the results of fcntl(2) apart.
---

diff --git a/locking.c b/locking.c
index e7acb6c..68cbc74 100644
--- a/locking.c
+++ b/locking.c
@@ -47,6 +47,7 @@
 /*----- Static variables --------------------------------------------------*/
 
 static jmp_buf jmp;
+static int err;
 
 /*----- Main code ---------------------------------------------------------*/
 
@@ -88,10 +89,11 @@ 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;
   time_t nt;
   pid_t kid;
@@ -167,24 +169,25 @@ 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));
+  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);
+  if ((fd = open(file, oflag, 0666)) < 0)
+    die(111, "error opening `%s': %s", file, strerror(errno));
+  err = fcntl(fd, f & f_wait ? F_SETLKW : F_SETLK, &l) >= 0 ? 0 : errno;
+done:
   signal(SIGALRM, oalrm);
   if (!ot)
     alarm(0);
@@ -193,12 +196,17 @@ doneopts:
     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);
-
+  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);
@@ -212,7 +220,7 @@ doneopts:
   l.l_start = 0;
   l.l_len = 0;
   fcntl(fd, F_SETLK, &l);
-  close(fd);
+  if (fd >= 0) close(fd);
   if (WIFEXITED(rc)) exit(WEXITSTATUS(rc));
   else exit(128 + WTERMSIG(rc));
 }