Fix up download a bit, better error checking and uses of long not double for timeout as curl needs, bug# 9748

Source/cmFileCommand.cxx

@@ -2465,17 +2465,48 @@ namespace{
     cmFileCommandVectorOfChar *vec
       = static_cast<cmFileCommandVectorOfChar*>(data);
     vec->insert(vec->end(), chPtr, chPtr + size);
-    
+
     return size;
   }
-  
+
-  
+
 }
 
 #endif
 
-bool 
+#if defined(CMAKE_BUILD_WITH_CMAKE)
-cmFileCommand::HandleDownloadCommand(std::vector<std::string> 
+namespace {
+
+  class cURLEasyGuard
+  {
+  public:
+    cURLEasyGuard(CURL * easy)
+      : Easy(easy)
+      {}
+    
+    ~cURLEasyGuard(void) 
+      {
+        if (this->Easy)
+          {
+          ::curl_easy_cleanup(this->Easy);
+          }
+      }
+    
+    inline void release(void) 
+      {
+        this->Easy = 0;
+        return;
+      }
+    
+  private:
+    ::CURL * Easy;
+  };
+  
+}
+#endif
+
+bool
+cmFileCommand::HandleDownloadCommand(std::vector<std::string>
                                      const& args)
 {
 #if defined(CMAKE_BUILD_WITH_CMAKE)
@@ -2486,25 +2517,25 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string>
                    "at least three arguments.");
     return false;
     }
-  i++; // Get rid of subcommand
+  ++i; // Get rid of subcommand
   std::string url = *i;
-  i++;
+  ++i;
   std::string file = *i;
-  i++;
+  ++i;
-  double timeout = 0;
+  long timeout = 0;
   std::string verboseLog;
   std::string statusVar;
   while(i != args.end())
     {
     if(*i == "TIMEOUT")
       {
-      i++;
+      ++i;
       if(i != args.end())
         {
-        timeout = atof(i->c_str());
+        timeout = atol(i->c_str());
         }
       else
-        { 
+        {
         this->SetError("FILE(DOWNLOAD url file TIMEOUT time) missing "
                        "time for TIMEOUT.");
         return false;
@@ -2512,7 +2543,7 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string>
       }
     else if(*i == "LOG")
       {
-      i++;
+      ++i;
       if( i == args.end())
         {
         this->SetError("FILE(DOWNLOAD url file LOG VAR) missing "
@@ -2523,7 +2554,7 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string>
       }
     else if(*i == "STATUS")
       {
-      i++;
+      ++i;
       if( i == args.end())
         {
         this->SetError("FILE(DOWNLOAD url file STATUS VAR) missing "
@@ -2532,7 +2563,7 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string>
         }
       statusVar = *i;
       }
-    i++;
+    ++i;
     }
 
   std::string dir = cmSystemTools::GetFilenamePath(file.c_str());
@@ -2553,50 +2584,106 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string>
                        "file for write.");
     return false;
     }
-  CURL *curl;
+  ::CURL *curl;
-  curl_global_init(CURL_GLOBAL_DEFAULT);
+  ::curl_global_init(CURL_GLOBAL_DEFAULT);
-  curl = curl_easy_init();
+  curl = ::curl_easy_init();
   if(!curl)
     {
     this->SetError("FILE(DOWNLOAD ) error "
                    "initializing curl.");
     return false;
     }
-  
+
-  curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
+  cURLEasyGuard g_curl(curl);
-  curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, 
+
-                   cmFileCommandWriteMemoryCallback);
+  ::CURLcode res = ::curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
-  curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION,
+  if (res != CURLE_OK)
-                   cmFileCommandCurlDebugCallback);
+    {
+      std::string errstring = "FILE(DOWNLOAD ) error; cannot set url: ";
+      errstring += ::curl_easy_strerror(res);
+    return false;
+    }
+
+  res = ::curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
+                             cmFileCommandWriteMemoryCallback);
+  if (res != CURLE_OK)
+      {
+      std::string errstring =
+        "FILE(DOWNLOAD ) error; cannot set write function: ";
+      errstring += ::curl_easy_strerror(res);
+    return false;
+    }
+
+  res = ::curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION,
+                             cmFileCommandCurlDebugCallback);
+  if (res != CURLE_OK)
+    {
+     std::string errstring =
+       "FILE(DOWNLOAD ) error; cannot set debug function: ";
+     errstring += ::curl_easy_strerror(res);
+    return false;
+    }
+
   cmFileCommandVectorOfChar chunkDebug;
-  ::curl_easy_setopt(curl, CURLOPT_FILE, (void *)&fout);
+
-  ::curl_easy_setopt(curl, CURLOPT_DEBUGDATA, (void *)&chunkDebug);
+  res = ::curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&fout);
+
+  if (res != CURLE_OK)
+    {
+    std::string errstring = "FILE(DOWNLOAD ) error; cannot set write data: ";
+    errstring += ::curl_easy_strerror(res);
+    return false;
+    }
+
+  res = ::curl_easy_setopt(curl, CURLOPT_DEBUGDATA, (void *)&chunkDebug);
+  if (res != CURLE_OK)
+    {
+    std::string errstring = "FILE(DOWNLOAD ) error; cannot set write data: ";
+    errstring += ::curl_easy_strerror(res);
+    return false;
+    }
+
   if(verboseLog.size())
     {
-    curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
+    res = ::curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
+
+    if (res != CURLE_OK)
+      {
+      std::string errstring = "FILE(DOWNLOAD ) error; cannot set verbose: ";
+      errstring += ::curl_easy_strerror(res);
+      return false;
+      }
     }
   if(timeout > 0)
     {
-    curl_easy_setopt(curl, CURLOPT_TIMEOUT, timeout ); 
+    res = ::curl_easy_setopt(curl, CURLOPT_TIMEOUT, timeout );
+
+    if (res != CURLE_OK)
+      {
+      std::string errstring = "FILE(DOWNLOAD ) error; cannot set verbose: ";
+      errstring += ::curl_easy_strerror(res);
+      return false;
+      }
     }
-  CURLcode res = curl_easy_perform(curl);
+  res = ::curl_easy_perform(curl);
   /* always cleanup */
-  curl_easy_cleanup(curl);
+  g_curl.release();
+  ::curl_easy_cleanup(curl);
   if(statusVar.size())
     {
     cmOStringStream result;
-    result << (int)res << ";\"" << curl_easy_strerror(res) << "\"";
+    result << (int)res << ";\"" << ::curl_easy_strerror(res) << "\"";
-    this->Makefile->AddDefinition(statusVar.c_str(), 
+    this->Makefile->AddDefinition(statusVar.c_str(),
                                   result.str().c_str());
     }
-  curl_global_cleanup();
+  ::curl_global_cleanup();
   if(chunkDebug.size())
     {
     chunkDebug.push_back(0);
     if(CURLE_OPERATION_TIMEOUTED == res)
-      { 
+      {
       std::string output = &*chunkDebug.begin();
-      
+
       if(verboseLog.size())
         {
         this->Makefile->AddDefinition(verboseLog.c_str(),
@@ -2608,9 +2695,9 @@ cmFileCommand::HandleDownloadCommand(std::vector<std::string>
                                   &*chunkDebug.begin());
     }
   return true;
-#else 
+#else
   this->SetError("FILE(DOWNLOAD ) "
                  "not supported in bootstrap cmake ");
   return false;
-#endif  
+#endif
 }

Source/cmFileCommand.h

@@ -152,7 +152,7 @@ public:
       "and the second element is a string value for the error. A 0 "
       "numeric error means no error in the operation. "
       "If TIMEOUT time is specified, the operation will "
-      "timeout after time seconds, time can be specified as a float."
+      "timeout after time seconds, time should be specified as an integer."
       "\n"
       "The file() command also provides COPY and INSTALL signatures:\n"
       "  file(<COPY|INSTALL> files... DESTINATION <dir>\n"