diff --git a/.clang-tidy b/.clang-tidy index bf72ae4..7162a41 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -29,7 +29,6 @@ Checks: ' -cppcoreguidelines-pro-bounds-array-to-pointer-decay, -cppcoreguidelines-pro-bounds-constant-array-index, -cppcoreguidelines-pro-bounds-pointer-arithmetic, - -cppcoreguidelines-pro-type-const-cast, -cppcoreguidelines-pro-type-member-init, -cppcoreguidelines-pro-type-reinterpret-cast, -cppcoreguidelines-pro-type-union-access, diff --git a/src/s3fs.cpp b/src/s3fs.cpp index 910ff9a..6032a72 100644 --- a/src/s3fs.cpp +++ b/src/s3fs.cpp @@ -145,9 +145,9 @@ static std::string raw_build_xattrs(const xattrs_t& xattrs); static std::string build_xattrs(const xattrs_t& xattrs); static int s3fs_check_service(); static bool set_mountpoint_attribute(struct stat& mpst); -static int set_bucket(const char* arg); +static int set_bucket(const std::string& arg); static int my_fuse_opt_proc(void* data, const char* arg, int key, struct fuse_args* outargs); -static fsblkcnt_t parse_bucket_size(char* value); +static fsblkcnt_t parse_bucket_size(std::string max_size); static bool is_cmd_exists(const std::string& command); static int print_umount_message(const std::string& mp, bool force) __attribute__ ((unused)); @@ -4553,32 +4553,31 @@ static bool set_mountpoint_attribute(struct stat& mpst) // // Set bucket and mount_prefix based on passed bucket name. // -static int set_bucket(const char* arg) +static int set_bucket(const std::string& arg) { - // TODO: Mutates input. Consider some other tokenization. - char *bucket_name = const_cast(arg); - if(strstr(arg, ":")){ - if(strstr(arg, "://")){ - S3FS_PRN_EXIT("bucket name and path(\"%s\") is wrong, it must be \"bucket[:/path]\".", arg); + size_t pos; + if((pos = arg.find(':')) != std::string::npos){ + if(arg.find("://") != std::string::npos){ + S3FS_PRN_EXIT("bucket name and path(\"%s\") is wrong, it must be \"bucket[:/path]\".", arg.c_str()); return -1; } - if(!S3fsCred::SetBucket(strtok(bucket_name, ":"))){ - S3FS_PRN_EXIT("bucket name and path(\"%s\") is wrong, it must be \"bucket[:/path]\".", arg); + std::string bucket_name = arg.substr(0, pos); + if(!S3fsCred::SetBucket(bucket_name)){ + S3FS_PRN_EXIT("bucket name and path(\"%s\") is wrong, it must be \"bucket[:/path]\".", arg.c_str()); return -1; } - char* pmount_prefix = strtok(nullptr, ""); - if(pmount_prefix){ - if(0 == strlen(pmount_prefix) || '/' != pmount_prefix[0]){ - S3FS_PRN_EXIT("path(%s) must be prefix \"/\".", pmount_prefix); - return -1; - } - mount_prefix = pmount_prefix; - // Trim the last consecutive '/' - mount_prefix = trim_right(mount_prefix, "/"); + + std::string pmount_prefix = arg.substr(pos + 1); + if(pmount_prefix.empty() || '/' != pmount_prefix[0]){ + S3FS_PRN_EXIT("path(%s) must be prefix \"/\".", pmount_prefix.c_str()); + return -1; } + mount_prefix = pmount_prefix; + // Trim the last consecutive '/' + mount_prefix = trim_right(mount_prefix, "/"); }else{ if(!S3fsCred::SetBucket(arg)){ - S3FS_PRN_EXIT("bucket name and path(\"%s\") is wrong, it must be \"bucket[:/path]\".", arg); + S3FS_PRN_EXIT("bucket name and path(\"%s\") is wrong, it must be \"bucket[:/path]\".", arg.c_str()); return -1; } } @@ -4593,70 +4592,70 @@ static int set_bucket(const char* arg) // of blocks with max_size calculated with the s3fs block size, // or 0 on error // -static fsblkcnt_t parse_bucket_size(char* max_size) +static fsblkcnt_t parse_bucket_size(std::string max_size) { const unsigned long long ten00 = 1000L; const unsigned long long ten24 = 1024L; unsigned long long scale = 1; unsigned long long n_bytes = 0; - char *ptr; + size_t pos; - if(nullptr != (ptr = strstr(max_size, "GB"))){ + if((pos = max_size.find("GB")) != std::string::npos){ scale = ten00 * ten00 * ten00; - if(2 < strlen(ptr)){ + if(2 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "GiB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("GiB")) != std::string::npos){ scale = ten24 * ten24 * ten24; - if(3 < strlen(ptr)){ + if(3 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "TB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("TB")) != std::string::npos){ scale = ten00 * ten00 * ten00 * ten00; - if(2 < strlen(ptr)){ + if(2 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "TiB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("TiB")) != std::string::npos){ scale = ten24 * ten24 * ten24 * ten24; - if(3 < strlen(ptr)){ + if(3 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "PB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("PB")) != std::string::npos){ scale = ten00 * ten00 * ten00 * ten00 * ten00; - if(2 < strlen(ptr)){ + if(2 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "PiB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("PiB")) != std::string::npos){ scale = ten24 * ten24 * ten24 * ten24 * ten24; - if(3 < strlen(ptr)){ + if(3 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "EB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("EB")) != std::string::npos){ scale = ten00 * ten00 * ten00 * ten00 * ten00 * ten00; - if(2 < strlen(ptr)){ + if(2 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; - }else if(nullptr != (ptr = strstr(max_size, "EiB"))){ + max_size.erase(pos); + }else if((pos = max_size.find("EiB")) != std::string::npos){ scale = ten24 * ten24 * ten24 * ten24 * ten24 * ten24; - if(3 < strlen(ptr)){ + if(3 < max_size.size() - pos){ return 0; // no trailing garbage } - *ptr = '\0'; + max_size.erase(pos); } // extra check - for(ptr = max_size; *ptr != '\0'; ++ptr){ - if(!isdigit(*ptr)){ + for(size_t i = 0; i < pos; ++i){ + if(!isdigit(max_size[i])){ return 0; // wrong number } - n_bytes = strtoull(max_size, nullptr, 10); + n_bytes = strtoull(max_size.c_str(), nullptr, 10); if((INT64_MAX / scale) < n_bytes){ return 0; // overflow } @@ -4720,7 +4719,7 @@ static int my_fuse_opt_proc(void* data, const char* arg, int key, struct fuse_ar if(key == FUSE_OPT_KEY_NONOPT){ // the first NONOPT option is the bucket name if(S3fsCred::GetBucket().empty()){ - if ((ret = set_bucket(arg))){ + if ((ret = set_bucket(arg)) != 0){ return ret; } return 0; @@ -4806,7 +4805,7 @@ static int my_fuse_opt_proc(void* data, const char* arg, int key, struct fuse_ar return 1; // continue for fuse option } else if(is_prefix(arg, "bucket_size=")){ - bucket_block_count = parse_bucket_size(const_cast(strchr(arg, '=')) + sizeof(char)); + bucket_block_count = parse_bucket_size(strchr(arg, '=') + sizeof(char)); if(0 == bucket_block_count){ S3FS_PRN_EXIT("invalid bucket_size option."); return -1; @@ -5052,7 +5051,7 @@ static int my_fuse_opt_proc(void* data, const char* arg, int key, struct fuse_ar } else if(is_prefix(arg, "bucket=")){ std::string bname = strchr(arg, '=') + sizeof(char); - if ((ret = set_bucket(bname.c_str()))){ + if ((ret = set_bucket(bname))){ return ret; } return 0; diff --git a/src/s3fs_cred.cpp b/src/s3fs_cred.cpp index a27d396..ebe574a 100644 --- a/src/s3fs_cred.cpp +++ b/src/s3fs_cred.cpp @@ -131,9 +131,9 @@ std::string S3fsCred::bucket_name; //------------------------------------------------------------------- // Class Methods //------------------------------------------------------------------- -bool S3fsCred::SetBucket(const char* bucket) +bool S3fsCred::SetBucket(const std::string& bucket) { - if(!bucket || strlen(bucket) == 0){ + if(bucket.empty()){ return false; } S3fsCred::bucket_name = bucket; diff --git a/src/s3fs_cred.h b/src/s3fs_cred.h index d842737..790cb45 100644 --- a/src/s3fs_cred.h +++ b/src/s3fs_cred.h @@ -158,7 +158,7 @@ class S3fsCred static bool CheckForbiddenBucketParams(); public: - static bool SetBucket(const char* bucket); + static bool SetBucket(const std::string& bucket); static const std::string& GetBucket(); S3fsCred(); diff --git a/src/s3fs_xml.cpp b/src/s3fs_xml.cpp index 2273e8f..c1fb55a 100644 --- a/src/s3fs_xml.cpp +++ b/src/s3fs_xml.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include "common.h" #include "s3fs_logger.h" @@ -30,11 +31,18 @@ #include "s3objlist.h" #include "string_util.h" +//------------------------------------------------------------------- +// Symbols +//------------------------------------------------------------------- +enum class get_object_name_result : std::uint8_t { + SUCCESS, + FAILURE, + FILE_OR_SUBDIR_IN_DIR +}; + //------------------------------------------------------------------- // Variables //------------------------------------------------------------------- -static constexpr char c_strErrorObjectName[] = "FILE or SUBDIR in DIR"; - // [NOTE] // mutex for static variables in GetXmlNsUrl // @@ -132,20 +140,17 @@ unique_ptr_xmlChar get_next_marker(xmlDocPtr doc) return get_base_exp(doc, "NextMarker"); } -// return: the pointer to object name on allocated memory. -// the pointer to "c_strErrorObjectName".(not allocated) -// nullptr(a case of something error occurred) -static char* get_object_name(xmlDocPtr doc, xmlNodePtr node, const char* path) +static std::pair get_object_name(xmlDocPtr doc, xmlNodePtr node, const char* path) { // Get full path unique_ptr_xmlChar fullpath(xmlNodeListGetString(doc, node, 1), xmlFree); if(!fullpath){ S3FS_PRN_ERR("could not get object full path name.."); - return nullptr; + return {get_object_name_result::FAILURE, ""}; } // basepath(path) is as same as fullpath. if(0 == strcmp(reinterpret_cast(fullpath.get()), path)){ - return const_cast(c_strErrorObjectName); + return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""}; } // Make dir path and filename @@ -156,31 +161,31 @@ static char* get_object_name(xmlDocPtr doc, xmlNodePtr node, const char* path) const char* basepath= (path && '/' == path[0]) ? &path[1] : path; if('\0' == mybname[0]){ - return nullptr; + return {get_object_name_result::FAILURE, ""}; } // check subdir & file in subdir if(0 < strlen(dirpath)){ // case of "/" if(0 == strcmp(mybname, "/") && 0 == strcmp(dirpath, "/")){ - return const_cast(c_strErrorObjectName); + return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""}; } // case of "." if(0 == strcmp(mybname, ".") && 0 == strcmp(dirpath, ".")){ - return const_cast(c_strErrorObjectName); + return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""}; } // case of ".." if(0 == strcmp(mybname, "..") && 0 == strcmp(dirpath, ".")){ - return const_cast(c_strErrorObjectName); + return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""}; } // case of "name" if(0 == strcmp(dirpath, ".")){ // OK - return strdup(mybname); + return {get_object_name_result::SUCCESS, mybname}; }else{ if(basepath && 0 == strcmp(dirpath, basepath)){ // OK - return strdup(mybname); + return {get_object_name_result::SUCCESS, mybname}; }else if(basepath && 0 < strlen(basepath) && '/' == basepath[strlen(basepath) - 1] && 0 == strncmp(dirpath, basepath, strlen(basepath) - 1)){ std::string withdirname; if(strlen(dirpath) > strlen(basepath)){ @@ -192,12 +197,12 @@ static char* get_object_name(xmlDocPtr doc, xmlNodePtr node, const char* path) withdirname += "/"; } withdirname += mybname; - return strdup(withdirname.c_str()); + return {get_object_name_result::SUCCESS, withdirname}; } } } // case of something wrong - return const_cast(c_strErrorObjectName); + return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""}; } static unique_ptr_xmlChar get_exp_value_xml(xmlDocPtr doc, xmlXPathContextPtr ctx, const char* exp_key) @@ -346,12 +351,13 @@ int append_objects_from_xml_ex(const char* path, xmlDocPtr doc, xmlXPathContextP continue; } xmlNodeSetPtr key_nodes = key->nodesetval; - char* name = get_object_name(doc, key_nodes->nodeTab[0]->xmlChildrenNode, path); + auto result = get_object_name(doc, key_nodes->nodeTab[0]->xmlChildrenNode, path); - if(!name){ + switch(result.first){ + case get_object_name_result::FAILURE: S3FS_PRN_WARN("name is something wrong. but continue."); - - }else if(reinterpret_cast(name) != c_strErrorObjectName){ + break; + case get_object_name_result::SUCCESS: { is_dir = isCPrefix ? true : false; stretag = ""; @@ -375,8 +381,7 @@ int append_objects_from_xml_ex(const char* path, xmlDocPtr doc, xmlXPathContextP // The XML data passed to this function is CR code(\r) encoded. // The function below decodes that encoded CR code. // - std::string decname = get_decoded_cr_code(name); - free(name); + std::string decname = get_decoded_cr_code(result.second.c_str()); if(prefix){ head.AddCommonPrefix(decname); @@ -385,8 +390,11 @@ int append_objects_from_xml_ex(const char* path, xmlDocPtr doc, xmlXPathContextP S3FS_PRN_ERR("insert_object returns with error."); return -1; } - }else{ + break; + } + case get_object_name_result::FILE_OR_SUBDIR_IN_DIR: S3FS_PRN_DBG("name is file or subdir in dir. but continue."); + break; } }