From f7e1a2a37f1467074db7828c77c784f4b8c0e734 Mon Sep 17 00:00:00 2001 From: "ggtakec@gmail.com" Date: Sat, 15 Jun 2013 15:29:08 +0000 Subject: [PATCH] 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 | 44 +++++++++++++++++++++++++++---------------- src/curl.h | 48 +++++++++++++++++++++++++++++++---------------- src/s3fs.cpp | 35 +++++++++++++++++++--------------- src/s3fs_util.cpp | 1 + 4 files changed, 81 insertions(+), 47 deletions(-) diff --git a/src/curl.cpp b/src/curl.cpp index 22014b7..9ef8ea2 100644 --- a/src/curl.cpp +++ b/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){ - pthread_mutex_lock(&curl_share_lock); + if(hCurlShare && useptr && CURL_LOCK_DATA_DNS == nLockData){ + 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){ - pthread_mutex_unlock(&curl_share_lock); + if(hCurlShare && useptr && CURL_LOCK_DATA_DNS == nLockData){ + 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) && 0 == result){ + if(hCurlShare && CURLSHE_OK != curl_share_cleanup(hCurlShare)){ + 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"){ diff --git a/src/curl.h b/src/curl.h index 12c1301..1637ff5 100644 --- a/src/curl.h +++ b/src/curl.h @@ -55,11 +55,36 @@ class auto_curl_slist { // header data struct head_data { - std::string base_path; - std::string path; - std::string *url; - struct curl_slist *requestHeaders; - headers_t *responseHeaders; + std::string* base_path; + std::string* path; + std::string* url; + struct curl_slist* requestHeaders; + 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 headMap_t; @@ -69,11 +94,7 @@ void destroy_curl_handle(CURL *curl_handle); struct cleanup_head_data { void operator()(std::pair qqq) { CURL* curl_handle = qqq.first; - - head_data response = qqq.second; - delete response.url; - curl_slist_free_all(response.requestHeaders); - delete response.responseHeaders; + (qqq.second).clear(); destroy_curl_handle(curl_handle); } }; @@ -92,13 +113,8 @@ class auto_head { if(iter == headMap.end()){ return; } - - head_data response = iter->second; - delete response.url; - curl_slist_free_all(response.requestHeaders); - delete response.responseHeaders; + (iter->second).clear(); destroy_curl_handle(curl_handle); - headMap.erase(iter); } diff --git a/src/s3fs.cpp b/src/s3fs.cpp index a9b11aa..6a41a3c 100644 --- a/src/s3fs.cpp +++ b/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.path = fullorg; + request_data.base_path = new string(*liter); + 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))){ - FGPRINT(" readdir_multi_head: failed adding stat cache [path=%s]\n", response.path.c_str()); + if(!StatCache::getStatCacheData()->AddStat((*response.path), (*response.responseHeaders))){ + 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_map.remove(msg->easy_handle); // with destroy curl handle. + curl_multi_remove_handle(mh, hSingle); + 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); } diff --git a/src/s3fs_util.cpp b/src/s3fs_util.cpp index 9006867..7eee3c1 100644 --- a/src/s3fs_util.cpp +++ b/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 }