From c346cf93958dad43cc4e778db8375dd390bbf1ce Mon Sep 17 00:00:00 2001
From: Paul Beckingham <paul@beckingham.net>
Date: Sat, 2 May 2015 11:47:39 -0400
Subject: [PATCH] Portability: Use fcntl instead of flock

- POSIX file locking mechanism, eliminating platform-specific code.
---
 ChangeLog    |  1 +
 src/File.cpp | 48 ++++++++++++++++++++----------------------------
 src/File.h   |  2 +-
 src/TDB2.cpp |  6 +++---
 src/util.cpp | 35 -----------------------------------
 src/util.h   |  9 ---------
 6 files changed, 25 insertions(+), 76 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a8aa1ceda..86b7c4c26 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,6 +7,7 @@
 - TW-1605 Japanese translation for Taskwarrior (thanks to Oota Toshiya).
 - TW-1606 scheduled.any filter (thanks to Peter Rochen).
 - The 'obfuscate' setting, if set to '1' will replace all text with 'xxx'.
+- POSIX file locking mechanism, eliminating platform-specific code.
 
 ------ current release ---------------------------
 
diff --git a/src/File.cpp b/src/File.cpp
index a922a299c..86ed71bb9 100644
--- a/src/File.cpp
+++ b/src/File.cpp
@@ -28,14 +28,10 @@
 #include <fstream>
 #include <sys/types.h>
 #include <sys/stat.h>
-#ifdef SOLARIS
-#include <fcntl.h> // for flock() replacement
-#include <string.h> // for memset()
-#else
-#include <sys/file.h>
-#endif
-#include <pwd.h>
+#include <stdio.h>
+#include <fcntl.h>
 #include <unistd.h>
+#include <pwd.h>
 #include <File.h>
 #include <text.h>
 #include <util.h>
@@ -151,6 +147,9 @@ void File::close ()
 {
   if (_fh)
   {
+    if (_locked)
+      unlock ();
+
     fclose (_fh);
     _fh = NULL;
     _h = -1;
@@ -161,38 +160,31 @@ void File::close ()
 ////////////////////////////////////////////////////////////////////////////////
 bool File::lock ()
 {
+  _locked = false;
   if (_fh && _h != -1)
   {
-    // Try three times before failing.
-    int retry = 0;
-    while (flock (_h, LOCK_NB | LOCK_EX) && ++retry <= 3)
-      ;
-
-    if (retry <= 3)
-    {
+                    // l_type   l_whence  l_start  l_len  l_pid
+    struct flock fl = {F_WRLCK, SEEK_SET, 0,       0,     0 };
+    fl.l_pid = getpid ();
+    if (fcntl (_h, F_SETLKW, &fl) == 0)
       _locked = true;
-      return true;
-    }
   }
 
-  _locked = false;
-  return false;
+  return _locked;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-bool File::waitForLock ()
+void File::unlock ()
 {
   if (_locked)
-    return true;
+  {
+                    // l_type   l_whence  l_start  l_len  l_pid
+    struct flock fl = {F_UNLCK, SEEK_SET, 0,       0,     0 };
+    fl.l_pid = getpid ();
 
-  if (_fh && _h != -1)
-    if (flock (_h, LOCK_EX) == 0)
-    {
-      _locked = true;
-      return true;
-    }
-
-  return false;
+    fcntl (_h, F_SETLK, &fl);
+    _locked = false;
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/File.h b/src/File.h
index a3d1df73c..007ca35a0 100644
--- a/src/File.h
+++ b/src/File.h
@@ -52,7 +52,7 @@ public:
   void close ();
 
   bool lock ();
-  bool waitForLock ();
+  void unlock ();
 
   void read (std::string&);
   void read (std::vector <std::string>&);
diff --git a/src/TDB2.cpp b/src/TDB2.cpp
index 3c02e565b..253fc6cca 100644
--- a/src/TDB2.cpp
+++ b/src/TDB2.cpp
@@ -226,7 +226,7 @@ void TF2::commit ()
       if (_file.open ())
       {
         if (context.config.getBoolean ("locking"))
-          _file.waitForLock ();
+          _file.lock ();
 
         // Write out all the added tasks.
         std::vector <Task>::iterator task;
@@ -258,7 +258,7 @@ void TF2::commit ()
       if (_file.open ())
       {
         if (context.config.getBoolean ("locking"))
-          _file.waitForLock ();
+          _file.lock ();
 
         // Truncate the file and rewrite.
         _file.truncate ();
@@ -358,7 +358,7 @@ void TF2::load_lines ()
   if (_file.open ())
   {
     if (context.config.getBoolean ("locking"))
-      _file.waitForLock ();
+      _file.lock ();
 
     _file.read (_lines);
     _file.close ();
diff --git a/src/util.cpp b/src/util.cpp
index 2e346da74..5711b0ab1 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -42,7 +42,6 @@
 #include <sys/types.h>
 #include <sys/time.h>
 #include <sys/wait.h>
-#include <fcntl.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>
@@ -228,40 +227,6 @@ const std::string uuid ()
 }
 #endif
 
-////////////////////////////////////////////////////////////////////////////////
-// On Solaris no flock function exists.
-#ifdef SOLARIS
-int flock (int fd, int operation)
-{
-  struct flock fl;
-
-  switch (operation & ~LOCK_NB)
-  {
-  case LOCK_SH:
-    fl.l_type = F_RDLCK;
-    break;
-
-  case LOCK_EX:
-    fl.l_type = F_WRLCK;
-    break;
-
-  case LOCK_UN:
-    fl.l_type = F_UNLCK;
-    break;
-
-  default:
-    errno = EINVAL;
-    return -1;
-  }
-
-  fl.l_whence = 0;
-  fl.l_start  = 0;
-  fl.l_len    = 0;
-
-  return fcntl (fd, (operation & LOCK_NB) ? F_SETLK : F_SETLKW, &fl);
-}
-#endif
-
 ////////////////////////////////////////////////////////////////////////////////
 // Run a binary with args, capturing output.
 int execute (
diff --git a/src/util.h b/src/util.h
index 766aab05e..843ac18db 100644
--- a/src/util.h
+++ b/src/util.h
@@ -52,15 +52,6 @@ const std::string uuid ();
 
 int execute (const std::string&, const std::vector <std::string>&, const std::string&, std::string&);
 
-#ifdef SOLARIS
-  #define LOCK_SH 1
-  #define LOCK_EX 2
-  #define LOCK_NB 4
-  #define LOCK_UN 8
-
-  int flock (int, int);
-#endif
-
 const std::string indentProject (
   const std::string&,
   const std::string& whitespace = "  ",