Fixed bugs

1) Fixed a bug(forgot removing temporary files)
    When s3fs gets a error from fwrite in multipart uploading function,
    s3fs does not remove a temporary file.

2) Fixed a bug(wrong prototype of function)
    The prototype of function for CURLSHOPT_UNLOCKFUNC
    is wrong.

3) Changed codes
    - In my_curl_easy_perform function, the codes for debugging messages
      is changed, because it is for not working codes when "-d" option is 
      not specified.
    - Changes struct head_data's member variables, and some codes for this 
      changes.
    - Moving calling function to main for curl_global_init and curl_share_init 
      functions, because these function must call in main thread.

4) Fixed a bug(use uninitialized memory)
    In get_lastmodified function, this function does not initialize value
   (struct tm).

5) Fixed a bug(access freed variable)
    In readdir_multi_head function, access a variable which is already freed.



git-svn-id: http://s3fs.googlecode.com/svn/trunk@442 df820570-a93a-0410-bd06-b72b767a4274

src/curl.cpp

@@ -172,15 +172,17 @@ void cleanup_curl_global_all(void)
 
 static void lock_curl_share(CURL* handle, curl_lock_data nLockData, curl_lock_access laccess, void* useptr)
 {
-  if(hCurlShare && CURL_LOCK_DATA_DNS == nLockData){
+  if(hCurlShare && useptr && CURL_LOCK_DATA_DNS == nLockData){
-    pthread_mutex_lock(&curl_share_lock);
+    pthread_mutex_t* lockmutex = (pthread_mutex_t*)useptr;
+    pthread_mutex_lock(lockmutex);
   }
 }
 
-static void unlock_curl_share(CURL* handle, curl_lock_data nLockData, curl_lock_access laccess, void* useptr)
+static void unlock_curl_share(CURL* handle, curl_lock_data nLockData, void* useptr)
 {
-  if(hCurlShare && CURL_LOCK_DATA_DNS == nLockData){
+  if(hCurlShare && useptr && CURL_LOCK_DATA_DNS == nLockData){
-    pthread_mutex_unlock(&curl_share_lock);
+    pthread_mutex_t* lockmutex = (pthread_mutex_t*)useptr;
+    pthread_mutex_unlock(lockmutex);
   }
 }
 
@@ -191,27 +193,34 @@ int init_curl_share(bool isCache)
   if(!isCache){
     return 0;
   }
+  pthread_mutex_init(&curl_share_lock, NULL);
+
   if(NULL == (hCurlShare = curl_share_init())){
     FGPRINT(" init_curl_share: curl_share_init failed\n");
     SYSLOGERR("init_curl_share: curl_share_init failed\n");
     return -1;
   }
   if(CURLSHE_OK != (nSHCode = curl_share_setopt(hCurlShare, CURLSHOPT_LOCKFUNC, lock_curl_share))){
-    FGPRINT(" init_curl_share: curl_share_setopt returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
+    FGPRINT(" init_curl_share: curl_share_setopt(LOCKFUNC) returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
     SYSLOGERR("init_curl_share: %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
     return nSHCode;
   }
   if(CURLSHE_OK != (nSHCode = curl_share_setopt(hCurlShare, CURLSHOPT_UNLOCKFUNC, unlock_curl_share))){
-    FGPRINT(" init_curl_share: curl_share_setopt returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
+    FGPRINT(" init_curl_share: curl_share_setopt(UNLOCKFUNC) returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
     SYSLOGERR("init_curl_share: %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
     return nSHCode;
   }
   if(CURLSHE_OK != (nSHCode = curl_share_setopt(hCurlShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS))){
-    FGPRINT(" init_curl_share: curl_share_setopt returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
+    FGPRINT(" init_curl_share: curl_share_setopt(DNS) returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
     SYSLOGERR("init_curl_share: %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
     return nSHCode;
   }
-  return pthread_mutex_init(&curl_share_lock, NULL);
+  if(CURLSHE_OK != (nSHCode = curl_share_setopt(hCurlShare, CURLSHOPT_USERDATA, (void*)&curl_share_lock))){
+    FGPRINT(" init_curl_share: curl_share_setopt(USERDATA) returns %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
+    SYSLOGERR("init_curl_share: %d(%s)\n", nSHCode, curl_share_strerror(nSHCode));
+    return nSHCode;
+  }
+  return 0;
 }
 
 int destroy_curl_share(bool isCache)
@@ -221,8 +230,10 @@ int destroy_curl_share(bool isCache)
   if(!isCache){
     return result;
   }
-  result = pthread_mutex_destroy(&curl_share_lock);
+  if(hCurlShare && CURLSHE_OK != curl_share_cleanup(hCurlShare)){
-  if(hCurlShare && CURLSHE_OK != curl_share_cleanup(hCurlShare) && 0 == result){
+    result = -1;
+  }
+  if(0 != pthread_mutex_destroy(&curl_share_lock)){
     result = -1;
   }
   return result;
@@ -396,7 +407,7 @@ int curl_get_headers(const char *path, headers_t &meta)
 CURL *create_head_handle(head_data *request_data)
 {
   CURL *curl_handle= create_curl_handle();
-  string realpath  = get_realpath(request_data->path.c_str());
+  string realpath  = get_realpath(request_data->path->c_str());
   string resource  = urlEncode(service_path + bucket + realpath);
   string url       = host + resource;
 
@@ -435,12 +446,13 @@ CURL *create_head_handle(head_data *request_data)
  */
 int my_curl_easy_perform(CURL* curl, BodyData* body, BodyData* head, FILE* f)
 {
-  char url[256];
   time_t now;
-  char* ptr_url = url;
-  curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL , &ptr_url);
 
-  SYSLOGDBG("connecting to URL %s", ptr_url);
+  if(debug){
+    char* ptr_url = NULL;
+    curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL , &ptr_url);
+    SYSLOGDBG("connecting to URL %s", ptr_url ? ptr_url : "unknown");
+  }
 
   // curl_easy_setopt(curl, CURLOPT_VERBOSE, true);
   if(ssl_verify_hostname.substr(0,1) == "0"){

src/curl.h

@@ -55,11 +55,36 @@ class auto_curl_slist {
 
 // header data
 struct head_data {
-  std::string base_path;
+  std::string* base_path;
-  std::string path;
+  std::string* path;
-  std::string *url;
+  std::string* url;
-  struct curl_slist *requestHeaders;
+  struct curl_slist* requestHeaders;
-  headers_t *responseHeaders;
+  headers_t*   responseHeaders;
+
+  head_data() : base_path(NULL), path(NULL), url(NULL), requestHeaders(NULL), responseHeaders(NULL) {}
+
+  void clear(void) {
+    if(base_path){
+      delete base_path;
+      base_path = NULL;
+    }
+    if(path){
+      delete path;
+      path = NULL;
+    }
+    if(url){
+      delete url;
+      url = NULL;
+    }
+    if(requestHeaders){
+      curl_slist_free_all(requestHeaders);
+      requestHeaders = NULL;
+    }
+    if(responseHeaders){
+      delete responseHeaders;
+      responseHeaders = NULL;
+    }
+  }
 };
 
 typedef std::map<CURL*, head_data> headMap_t;
@@ -69,11 +94,7 @@ void destroy_curl_handle(CURL *curl_handle);
 struct cleanup_head_data {
   void operator()(std::pair<CURL*, head_data> qqq) {
     CURL* curl_handle  = qqq.first;
-
+    (qqq.second).clear();
-    head_data response = qqq.second;
-    delete response.url;
-    curl_slist_free_all(response.requestHeaders);
-    delete response.responseHeaders;
     destroy_curl_handle(curl_handle);
   }
 };
@@ -92,13 +113,8 @@ class auto_head {
     if(iter == headMap.end()){
       return;
     }
-
+    (iter->second).clear();
-    head_data response = iter->second;
-    delete response.url;
-    curl_slist_free_all(response.requestHeaders);
-    delete response.responseHeaders;
     destroy_curl_handle(curl_handle);
-
     headMap.erase(iter);
   }
 

src/s3fs.cpp

@@ -1096,6 +1096,7 @@ static int put_local_fd_big_file(const char* path, headers_t meta, int fd, bool
           if(buffer){
             free(buffer);
           }
+          remove(part.path);
           return(-EIO);
         }
       }
@@ -2975,11 +2976,12 @@ static int readdir_multi_head(const char *path, S3ObjList& head)
 
       // file not cached, prepare a call to get_headers
       head_data request_data;
-      request_data.base_path      = (*liter);
+      request_data.base_path      = new string(*liter);
-      request_data.path           = fullorg;
+      request_data.path           = new string(fullorg);
       CURL* curl_handle           = create_head_handle(&request_data);
       my_set_curl_share(curl_handle);  // set dns cache
-      request_data.path           = fullpath;    // Notice: replace org to normalized for cache key.
+      delete request_data.path;
+      request_data.path           = new string(fullpath);    // Notice: replace org to normalized for cache key.
       curl_map.get()[curl_handle] = request_data;
 
       // add this handle to the multi handle
@@ -3051,22 +3053,23 @@ static int readdir_multi_head(const char *path, S3ObjList& head)
         curl_multi_cleanup(mh);
         return -EIO;
       }
+      CURL* hSingle = msg->easy_handle;
 
       if(CURLE_OK == msg->data.result){
-        head_data response= curl_map.get()[msg->easy_handle];
+        head_data response= curl_map.get()[hSingle];
         long responseCode = -1;
 
-        if(CURLE_OK == curl_easy_getinfo(msg->easy_handle, CURLINFO_RESPONSE_CODE, &responseCode) && 400 > responseCode){
+        if(CURLE_OK == curl_easy_getinfo(hSingle, CURLINFO_RESPONSE_CODE, &responseCode) && 400 > responseCode){
           // add into stat cache
-          if(!StatCache::getStatCacheData()->AddStat(response.path, (*response.responseHeaders))){
+          if(!StatCache::getStatCacheData()->AddStat((*response.path), (*response.responseHeaders))){
-            FGPRINT("  readdir_multi_head: failed adding stat cache [path=%s]\n", response.path.c_str());
+            FGPRINT("  readdir_multi_head: failed adding stat cache [path=%s]\n", response.path->c_str());
           }
         }else{
           // This case is directory object("dir", "non dir object", "_$folder$", etc)
-          //FGPRINT("  readdir_multi_head: failed a request(%s)\n", response.base_path.c_str());
+          //FGPRINT("  readdir_multi_head: failed a request(%s)\n", response.base_path->c_str());
         }
         // remove request path.
-        headlist.remove(response.base_path);
+        headlist.remove((*response.base_path));
 
       }else{
         SYSLOGDBGERR("readdir_multi_head: failed to read - remaining_msgs: %i code: %d  msg: %s", 
@@ -3076,8 +3079,8 @@ static int readdir_multi_head(const char *path, S3ObjList& head)
       }
 
       // Cleanup this curl handle and headers
-      curl_multi_remove_handle(mh, msg->easy_handle);
+      curl_multi_remove_handle(mh, hSingle);
-      curl_map.remove(msg->easy_handle);  // with destroy curl handle.
+      curl_map.remove(hSingle);  // with destroy curl handle.
     }
     curl_multi_cleanup(mh);
     curl_map.removeAll();  // with destroy curl handle.
@@ -3505,10 +3508,8 @@ static void* s3fs_init(struct fuse_conn_info *conn)
   }
   CRYPTO_set_locking_callback(locking_function);
   CRYPTO_set_id_callback(id_function);
-  init_curl_global_all();
   init_curl_handles_mutex();
   InitMimeType("/etc/mime.types");
-  init_curl_share(dns_cache);
 
   // Investigate system capabilities
   if((unsigned int)conn->capable & FUSE_CAP_ATOMIC_O_TRUNC){
@@ -3530,8 +3531,6 @@ static void s3fs_destroy(void*)
   }
   free(mutex_buf);
   mutex_buf = NULL;
-  destroy_curl_share(dns_cache);
-  cleanup_curl_global_all();
   destroy_curl_handles_mutex();
 }
 
@@ -4584,10 +4583,16 @@ int main(int argc, char *argv[]) {
   s3fs_oper.access = s3fs_access;
   s3fs_oper.create = s3fs_create;
 
+  init_curl_global_all();
+  init_curl_share(dns_cache);
+
   // now passing things off to fuse, fuse will finish evaluating the command line args
   fuse_res = fuse_main(custom_args.argc, custom_args.argv, &s3fs_oper, NULL);
   fuse_opt_free_args(&custom_args);
 
+  destroy_curl_share(dns_cache);
+  cleanup_curl_global_all();
+
   exit(fuse_res);
 }
 

src/s3fs_util.cpp

@@ -623,6 +623,7 @@ time_t get_lastmodified(const char* s)
   if(!s){
     return 0L;
   }
+  memset(&tm, 0, sizeof(struct tm));
   strptime(s, "%a, %d %b %Y %H:%M:%S %Z", &tm);
   return mktime(&tm);      // GMT
 }