Enable cppcoreguidelines-pro-type-const-cast (#2537)

This fixes dangerous uses of const_cast.

.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,

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<char*>(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);
+
+        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<char *>(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;

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;

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();

src/s3fs_xml.cpp

@@ -22,6 +22,7 @@
 #include <cstdlib>
 #include <libxml/xpathInternals.h>
 #include <mutex>
+#include <string>
 
 #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_result, std::string> 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<char*>(fullpath.get()), path)){
-        return const_cast<char*>(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<char*>(c_strErrorObjectName);
+            return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""};
         }
         // case of "."
         if(0 == strcmp(mybname, ".") && 0 == strcmp(dirpath, ".")){
-            return const_cast<char *>(c_strErrorObjectName);
+            return {get_object_name_result::FILE_OR_SUBDIR_IN_DIR, ""};
         }
         // case of ".."
         if(0 == strcmp(mybname, "..") && 0 == strcmp(dirpath, ".")){
-            return const_cast<char *>(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<char*>(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<const char*>(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;
         }
     }