On 05-Dec-14 17:09, Brad King wrote:
On 12/05/2014 09:03 AM, Ruslan Baratov wrote:
Actually why not use 'strtoll' and 'long long' ?
I'm not sure that function or type exists portably on some of the
older host platforms we support.  The "long" type should be plenty
big for timeout values, and there are other strtol call sites that
already use "long" and could be converted to use a StringToLong
helper too.
Sending patch with 'long'.
Actually for the lock timeout use case would it make sense to change
to a floating point value so timeouts can be more granular?  Some
use cases may not want to block for a whole second between lock
attempts.

What granularity you think is appropriate, 100 ms? 10 ms? (It will be value in `cmSystemTools::Delay(...);`)

Ruslo
>From 3ed0ad891c6108fa2a03834f9fe109ee5d628847 Mon Sep 17 00:00:00 2001
From: Ruslan Baratov <ruslan_bara...@yahoo.com>
Date: Fri, 5 Dec 2014 17:18:11 +0300
Subject: [PATCH] Use 'long' for StringToInt

---
 Source/cmFileCommand.cxx   |  6 +++---
 Source/cmFileLock.cxx      |  4 ++--
 Source/cmFileLock.h        |  4 ++--
 Source/cmFileLockPool.cxx  |  8 ++++----
 Source/cmFileLockPool.h    | 10 ++++++----
 Source/cmFileLockUnix.cxx  |  2 +-
 Source/cmFileLockWin32.cxx |  2 +-
 Source/cmSystemTools.cxx   |  7 ++++---
 Source/cmSystemTools.h     |  4 ++--
 9 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index a6eb8c4..42cd132 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -3521,7 +3521,7 @@ bool cmFileCommand::HandleLockCommand(
   };
   Guard guard = GUARD_PROCESS;
   std::string resultVariable;
-  unsigned timeout = static_cast<unsigned>(-1);
+  unsigned long timeout = static_cast<unsigned long>(-1);
 
   // Parse arguments
   if(args.size() < 2)
@@ -3597,7 +3597,7 @@ bool cmFileCommand::HandleLockCommand(
             "expected timeout value after TIMEOUT");
         return false;
         }
-      int scanned;
+      long scanned;
       if(!cmSystemTools::StringToInt(args[i].c_str(), &scanned) || scanned < 0)
         {
         cmOStringStream e;
@@ -3605,7 +3605,7 @@ bool cmFileCommand::HandleLockCommand(
         this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
         return false;
         }
-      timeout = static_cast<unsigned>(scanned);
+      timeout = static_cast<unsigned long>(scanned);
       }
     else
       {
diff --git a/Source/cmFileLock.cxx b/Source/cmFileLock.cxx
index 5f75637..e6aa5f4 100644
--- a/Source/cmFileLock.cxx
+++ b/Source/cmFileLock.cxx
@@ -28,7 +28,7 @@ cmFileLock::~cmFileLock()
 }
 
 cmFileLockResult cmFileLock::Lock(
-    const std::string& filename, unsigned timeout)
+    const std::string& filename, unsigned long timeout)
 {
   if (filename.empty())
     {
@@ -48,7 +48,7 @@ cmFileLockResult cmFileLock::Lock(
   cmFileLockResult result = this->OpenFile();
   if (result.IsOk())
     {
-    if (timeout == static_cast<unsigned>(-1))
+    if (timeout == static_cast<unsigned long>(-1))
       {
       result = this->LockWithoutTimeout();
       }
diff --git a/Source/cmFileLock.h b/Source/cmFileLock.h
index 4d922a0..dd959a7 100644
--- a/Source/cmFileLock.h
+++ b/Source/cmFileLock.h
@@ -37,7 +37,7 @@ class cmFileLock
     * @brief Lock the file.
     * @param timeoutSec Lock timeout. If -1 try until success or fatal error.
     */
-  cmFileLockResult Lock(const std::string& filename, unsigned timeoutSec);
+  cmFileLockResult Lock(const std::string& filename, unsigned long timeoutSec);
 
   /**
     * @brief Unlock the file.
@@ -57,7 +57,7 @@ class cmFileLock
 
   cmFileLockResult OpenFile();
   cmFileLockResult LockWithoutTimeout();
-  cmFileLockResult LockWithTimeout(unsigned timeoutSec);
+  cmFileLockResult LockWithTimeout(unsigned long timeoutSec);
 
 #if defined(_WIN32)
   typedef HANDLE FileId;
diff --git a/Source/cmFileLockPool.cxx b/Source/cmFileLockPool.cxx
index e84e71a..551a75a 100644
--- a/Source/cmFileLockPool.cxx
+++ b/Source/cmFileLockPool.cxx
@@ -60,7 +60,7 @@ void cmFileLockPool::PopFileScope()
 }
 
 cmFileLockResult cmFileLockPool::LockFunctionScope(
-    const std::string& filename, unsigned timeoutSec)
+    const std::string& filename, unsigned long timeoutSec)
 {
   if (this->IsAlreadyLocked(filename))
     {
@@ -74,7 +74,7 @@ cmFileLockResult cmFileLockPool::LockFunctionScope(
 }
 
 cmFileLockResult cmFileLockPool::LockFileScope(
-    const std::string& filename, unsigned timeoutSec)
+    const std::string& filename, unsigned long timeoutSec)
 {
   if (this->IsAlreadyLocked(filename))
     {
@@ -85,7 +85,7 @@ cmFileLockResult cmFileLockPool::LockFileScope(
 }
 
 cmFileLockResult cmFileLockPool::LockProcessScope(
-    const std::string& filename, unsigned timeoutSec)
+    const std::string& filename, unsigned long timeoutSec)
 {
   if (this->IsAlreadyLocked(filename))
     {
@@ -155,7 +155,7 @@ cmFileLockPool::ScopePool::~ScopePool()
 }
 
 cmFileLockResult cmFileLockPool::ScopePool::Lock(
-    const std::string& filename, unsigned timeoutSec)
+    const std::string& filename, unsigned long timeoutSec)
 {
   cmFileLock *lock = new cmFileLock();
   const cmFileLockResult result = lock->Lock(filename, timeoutSec);
diff --git a/Source/cmFileLockPool.h b/Source/cmFileLockPool.h
index a63540c..baef310 100644
--- a/Source/cmFileLockPool.h
+++ b/Source/cmFileLockPool.h
@@ -45,13 +45,13 @@ class cmFileLockPool
     * @param timeoutSec Lock timeout. If -1 try until success or fatal error.
     */
   cmFileLockResult LockFunctionScope(
-      const std::string& filename, unsigned timeoutSec
+      const std::string& filename, unsigned long timeoutSec
   );
   cmFileLockResult LockFileScope(
-      const std::string& filename, unsigned timeoutSec
+      const std::string& filename, unsigned long timeoutSec
   );
   cmFileLockResult LockProcessScope(
-      const std::string& filename, unsigned timeoutSec
+      const std::string& filename, unsigned long timeoutSec
   );
   //@}
 
@@ -72,7 +72,9 @@ class cmFileLockPool
     ScopePool();
     ~ScopePool();
 
-    cmFileLockResult Lock(const std::string& filename, unsigned timeoutSec);
+    cmFileLockResult Lock(
+        const std::string& filename, unsigned long timeoutSec
+    );
     cmFileLockResult Release(const std::string& filename);
     bool IsAlreadyLocked(const std::string& filename) const;
 
diff --git a/Source/cmFileLockUnix.cxx b/Source/cmFileLockUnix.cxx
index 038011e..fc18a64 100644
--- a/Source/cmFileLockUnix.cxx
+++ b/Source/cmFileLockUnix.cxx
@@ -66,7 +66,7 @@ cmFileLockResult cmFileLock::LockWithoutTimeout()
     }
 }
 
-cmFileLockResult cmFileLock::LockWithTimeout(unsigned seconds)
+cmFileLockResult cmFileLock::LockWithTimeout(unsigned long seconds)
 {
   while (true)
     {
diff --git a/Source/cmFileLockWin32.cxx b/Source/cmFileLockWin32.cxx
index 17231ea..4691689 100644
--- a/Source/cmFileLockWin32.cxx
+++ b/Source/cmFileLockWin32.cxx
@@ -86,7 +86,7 @@ cmFileLockResult cmFileLock::LockWithoutTimeout()
     }
 }
 
-cmFileLockResult cmFileLock::LockWithTimeout(unsigned seconds)
+cmFileLockResult cmFileLock::LockWithTimeout(unsigned long seconds)
 {
   const DWORD flags = LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY;
   while (true)
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index 9852dd6..09a3452 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -2925,9 +2925,10 @@ std::vector<std::string> cmSystemTools::tokenize(const 
std::string& str,
 }
 
 //----------------------------------------------------------------------------
-bool cmSystemTools::StringToInt(const char* str, int* value)
+bool cmSystemTools::StringToInt(const char* str, long* value)
 {
+  errno = 0;
   char *endp;
-  *value = static_cast<int>(strtol(str, &endp, 10));
-  return (*endp == '\0') && (endp != str);
+  *value = strtol(str, &endp, 10);
+  return (*endp == '\0') && (endp != str) && (errno == 0);
 }
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 763389b..2687bd9 100644
--- a/Source/cmSystemTools.h
+++ b/Source/cmSystemTools.h
@@ -458,8 +458,8 @@ public:
   static std::vector<std::string> tokenize(const std::string& str,
                                            const std::string& sep);
 
-  /** Convert string to int. Expected that the whole string is an integer */
-  static bool StringToInt(const char* str, int* value);
+  /** Convert string to long. Expected that the whole string is an integer */
+  static bool StringToInt(const char* str, long* value);
 
 #ifdef _WIN32
   struct WindowsFileRetry
-- 
2.1.1

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to