On 01-Dec-14 23:32, Brad King wrote:
On 12/01/2014 03:26 PM, Stephen Kelly wrote:
Existing uses of strtol also check errno. I guess your implementation should
too.
Yes, IIUC it is a range check on whether the value can be represented.
I think also you need to zero errno before strtol since errno is not zeroed on successful conversion.

Then the existing users should use this new method.
In that case the type should be changed to 'long' instead of 'int',
and the current call site should be updated accordingly.
Actually why not use 'strtoll' and 'long long' ?

Sending patch with 'strtoll', 'long long' and 'errno=0'.

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

---
 Source/cmFileCommand.cxx   |  6 +++---
 Source/cmFileLock.cxx      |  4 ++--
 Source/cmFileLock.h        |  6 ++++--
 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, 27 insertions(+), 22 deletions(-)

diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index a6eb8c4..c86b768 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 long timeout = static_cast<unsigned long 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 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 long>(scanned);
       }
     else
       {
diff --git a/Source/cmFileLock.cxx b/Source/cmFileLock.cxx
index 5f75637..f891105 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 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 long>(-1))
       {
       result = this->LockWithoutTimeout();
       }
diff --git a/Source/cmFileLock.h b/Source/cmFileLock.h
index 4d922a0..7a8e6db 100644
--- a/Source/cmFileLock.h
+++ b/Source/cmFileLock.h
@@ -37,7 +37,9 @@ 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 long timeoutSec
+  );
 
   /**
     * @brief Unlock the file.
@@ -57,7 +59,7 @@ class cmFileLock
 
   cmFileLockResult OpenFile();
   cmFileLockResult LockWithoutTimeout();
-  cmFileLockResult LockWithTimeout(unsigned timeoutSec);
+  cmFileLockResult LockWithTimeout(unsigned long long timeoutSec);
 
 #if defined(_WIN32)
   typedef HANDLE FileId;
diff --git a/Source/cmFileLockPool.cxx b/Source/cmFileLockPool.cxx
index e84e71a..862f44c 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 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 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 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 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..1d7d73c 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 long timeoutSec
   );
   cmFileLockResult LockFileScope(
-      const std::string& filename, unsigned timeoutSec
+      const std::string& filename, unsigned long long timeoutSec
   );
   cmFileLockResult LockProcessScope(
-      const std::string& filename, unsigned timeoutSec
+      const std::string& filename, unsigned long 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 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..d9af0b0 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 long seconds)
 {
   while (true)
     {
diff --git a/Source/cmFileLockWin32.cxx b/Source/cmFileLockWin32.cxx
index 17231ea..457f2df 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 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..def2c26 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 long* value)
 {
+  errno = 0;
   char *endp;
-  *value = static_cast<int>(strtol(str, &endp, 10));
-  return (*endp == '\0') && (endp != str);
+  *value = strtoll(str, &endp, 10);
+  return (*endp == '\0') && (endp != str) && (errno == 0);
 }
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 763389b..b4d3177 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 integer. Expected that the whole string is an integer 
*/
+  static bool StringToInt(const char* str, long 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