Apply clang-tidy to headers (#2470)

.clang-tidy

@@ -32,6 +32,7 @@ Checks: '
     -cppcoreguidelines-pro-type-reinterpret-cast,
     -cppcoreguidelines-pro-type-union-access,
     -cppcoreguidelines-pro-type-vararg,
+    -cppcoreguidelines-special-member-functions,
     google-*,
     -google-build-using-namespace,
     -google-readability-casting,
@@ -43,6 +44,7 @@ Checks: '
     -misc-const-correctness,
     -misc-include-cleaner,
     -misc-no-recursion,
+    -misc-non-private-member-variables-in-classes,
     -misc-redundant-expression,
     -misc-unused-parameters,
     -misc-use-anonymous-namespace,
@@ -50,10 +52,12 @@ Checks: '
     -modernize-avoid-c-arrays,
     -modernize-loop-convert,
     -modernize-make-unique,
+    -modernize-use-nodiscard,
     -modernize-raw-string-literal,
     -modernize-return-braced-init-list,
     -modernize-use-auto,
     -modernize-use-default-member-init,
+    -modernize-use-equals-delete,
     -modernize-use-trailing-return-type,
     -modernize-use-using,
     performance-*,
@@ -72,6 +76,7 @@ Checks: '
     -readability-isolate-declaration,
     -readability-magic-numbers,
     -readability-named-parameter,
+    -readability-redundant-access-specifiers,
     -readability-redundant-declaration,
     -readability-simplify-boolean-expr,
     -readability-suspicious-call-argument'

src/Makefile.am

@@ -102,7 +102,7 @@ TESTS = \
     test_string_util
 
 clang-tidy:
-	clang-tidy $(s3fs_SOURCES) -- $(DEPS_CFLAGS) $(CPPFLAGS)
+	clang-tidy -extra-arg-before=-xc++ *.h $(s3fs_SOURCES) -- $(DEPS_CFLAGS) $(CPPFLAGS)
 
 #
 # Local variables:

src/autolock.h

@@ -21,6 +21,7 @@
 #ifndef S3FS_AUTOLOCK_H_
 #define S3FS_AUTOLOCK_H_
 
+#include <cstdint>
 #include <pthread.h>
 
 // empty annotation to indicate lock requirement
@@ -32,7 +33,7 @@
 class AutoLock
 {
     public:
-      enum Type {
+      enum Type : uint8_t {
           NO_WAIT        = 1,
           ALREADY_LOCKED = 2,
           NONE           = 0

src/cache.cpp

@@ -21,6 +21,7 @@
 #include <algorithm>
 #include <cerrno>
 #include <cstdlib>
+#include <sys/stat.h>
 #include <vector>
 
 #include "s3fs.h"

src/cache.h

@@ -22,6 +22,10 @@
 #define S3FS_CACHE_H_
 
 #include <cstring>
+#include <map>
+#include <string>
+#include <sys/stat.h>
+#include <vector>
 
 #include "autolock.h"
 #include "metaheader.h"
@@ -34,19 +38,16 @@
 //
 struct stat_cache_entry {
     struct stat       stbuf;
-    unsigned long     hit_count;
+    unsigned long     hit_count = 0;
-    struct timespec   cache_date;
+    struct timespec   cache_date = {0, 0};
     headers_t         meta;
-    bool              isforce;
+    bool              isforce = false;
-    bool              noobjcache;  // Flag: cache is no object for no listing.
+    bool              noobjcache = false;  // Flag: cache is no object for no listing.
-    unsigned long     notruncate;  // 0<:   not remove automatically at checking truncate
+    unsigned long     notruncate = 0L;  // 0<:   not remove automatically at checking truncate
 
-    stat_cache_entry() : hit_count(0), isforce(false), noobjcache(false), notruncate(0L)
+    stat_cache_entry()
     {
-        memset(&stbuf, 0, sizeof(struct stat));
+        memset(&stbuf, 0, sizeof(stbuf));
-        cache_date.tv_sec  = 0;
-        cache_date.tv_nsec = 0;
-        meta.clear();
     }
 };
 
@@ -57,14 +58,8 @@ typedef std::map<std::string, stat_cache_entry> stat_cache_t; // key=path
 //
 struct symlink_cache_entry {
     std::string       link;
-    unsigned long     hit_count;
+    unsigned long     hit_count = 0;
-    struct timespec   cache_date;  // The function that operates timespec uses the same as Stats
+    struct timespec   cache_date = {0, 0};  // The function that operates timespec uses the same as Stats
-
-    symlink_cache_entry() : link(""), hit_count(0)
-    {
-      cache_date.tv_sec  = 0;
-      cache_date.tv_nsec = 0;
-    }
 };
 
 typedef std::map<std::string, symlink_cache_entry> symlink_cache_t;

src/curl.h

@@ -21,14 +21,17 @@
 #ifndef S3FS_CURL_H_
 #define S3FS_CURL_H_
 
+#include <cstdint>
 #include <curl/curl.h>
 #include <map>
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "autolock.h"
-#include "metaheader.h"
 #include "fdcache_page.h"
+#include "metaheader.h"
+#include "types.h"
 
 //----------------------------------------------
 // Avoid dependency on libcurl version
@@ -91,7 +94,7 @@ class S3fsCurl
     friend class S3fsMultiCurl;
 
     private:
-        enum class REQTYPE {
+        enum class REQTYPE : int8_t {
             UNSET  = -1,
             DELETE = 0,
             HEAD,

src/curl_util.h

@@ -21,9 +21,11 @@
 #ifndef S3FS_CURL_UTIL_H_
 #define S3FS_CURL_UTIL_H_
 
+#include <cstdint>
 #include <curl/curl.h>
+#include <string>
 
-enum class sse_type_t;
+enum class sse_type_t : uint8_t;
 
 //----------------------------------------------
 // Functions

src/fdcache_entity.h

@@ -21,6 +21,7 @@
 #ifndef S3FS_FDCACHE_ENTITY_H_
 #define S3FS_FDCACHE_ENTITY_H_
 
+#include <cstdint>
 #include <fcntl.h>
 #include <memory>
 
@@ -41,7 +42,7 @@ class FdEntity
         // because the processing(request) at these updates is different.
         // Therefore, the pending state is expressed by this enum type.
         //
-        enum class pending_status_t {
+        enum class pending_status_t : uint8_t {
             NO_UPDATE_PENDING = 0,
             UPDATE_META_PENDING,        // pending meta header
             CREATE_FILE_PENDING         // pending file creation and meta header

src/fdcache_fdinfo.h

@@ -38,17 +38,15 @@ class PseudoFdInfo;
 
 struct pseudofdinfo_thparam
 {
-    PseudoFdInfo* ppseudofdinfo;
+    PseudoFdInfo* ppseudofdinfo = nullptr;
     std::string   path;
     std::string   upload_id;
-    int           upload_fd;
+    int           upload_fd = -1;
-    off_t         start;
+    off_t         start = 0;
-    off_t         size;
+    off_t         size = 0;
-    bool          is_copy;
+    bool          is_copy = false;
-    int           part_num;
+    int           part_num = -1;
-    etagpair*     petag;
+    etagpair*     petag = nullptr;
-
-    pseudofdinfo_thparam() : ppseudofdinfo(nullptr), path(""), upload_id(""), upload_fd(-1), start(0), size(0), is_copy(false), part_num(-1), petag(nullptr) {}
 };
 
 //------------------------------------------------

src/fdcache_page.h

@@ -21,6 +21,7 @@
 #ifndef S3FS_FDCACHE_PAGE_H_
 #define S3FS_FDCACHE_PAGE_H_
 
+#include <cstdint>
 #include <sys/types.h>
 #include <vector>
 
@@ -79,7 +80,7 @@ class PageList
         bool          is_shrink;    // [NOTE] true if it has been shrinked even once
 
     public:
-        enum class page_status{
+        enum class page_status : int8_t {
             NOT_LOAD_MODIFIED = 0,
             LOADED,
             MODIFIED,

src/fdcache_pseudofd.h

@@ -21,6 +21,7 @@
 #ifndef S3FS_FDCACHE_PSEUDOFD_H_
 #define S3FS_FDCACHE_PSEUDOFD_H_
 
+#include <pthread.h>
 #include <vector>
 
 //------------------------------------------------

src/metaheader.h

@@ -21,9 +21,10 @@
 #ifndef S3FS_METAHEADER_H_
 #define S3FS_METAHEADER_H_
 
+#include <map>
 #include <string>
 #include <strings.h>
-#include <map>
+#include <sys/stat.h>
 
 //-------------------------------------------------------------------
 // headers_t

src/mpu_util.h

@@ -21,6 +21,8 @@
 #ifndef S3FS_MPU_UTIL_H_
 #define S3FS_MPU_UTIL_H_
 
+#include <cstdint>
+#include <ctime>
 #include <string>
 #include <vector>
 
@@ -39,7 +41,7 @@ typedef std::vector<INCOMP_MPU_INFO> incomp_mpu_list_t;
 //-------------------------------------------------------------------
 // enum for utility process mode
 //-------------------------------------------------------------------
-enum class utility_incomp_type{
+enum class utility_incomp_type : uint8_t {
     NO_UTILITY_MODE = 0,      // not utility mode
     INCOMP_TYPE_LIST,         // list of incomplete mpu
     INCOMP_TYPE_ABORT         // delete incomplete mpu

src/psemaphore.h

@@ -59,13 +59,13 @@ class Semaphore
         int get_value() const { return value; }
 
     private:
-        const int value;
+        int value;
         dispatch_semaphore_t sem;
 };
 
 #else
 
-#include <errno.h>
+#include <cerrno>
 #include <semaphore.h>
 
 class Semaphore
@@ -93,7 +93,7 @@ class Semaphore
         int get_value() const { return value; }
 
     private:
-        const int value;
+        int value;
         sem_t mutex;
 };
 

src/s3fs.cpp

@@ -3363,7 +3363,7 @@ static int readdir_multi_head(const char* path, const S3ObjList& head, void* buf
     // as a path, so search for objects under that path.(a case of no dir object)
     //
     if(!support_compat_dir){
-        syncfiller.SufficiencyFill(head.common_prefixes);
+        syncfiller.SufficiencyFill(head.GetCommonPrefixes());
     }
     if(support_compat_dir && !notfound_param.notfound_list.empty()){      // [NOTE] not need to lock to access this here.
         // dummy header

src/s3fs_cred.h

@@ -21,8 +21,12 @@
 #ifndef S3FS_CRED_H_
 #define S3FS_CRED_H_
 
+#include <map>
+#include <string>
+
 #include "autolock.h"
 #include "s3fs_extcred.h"
+#include "types.h"
 
 //----------------------------------------------
 // Typedefs

src/s3fs_logger.h

@@ -22,6 +22,7 @@
 #define S3FS_LOGGER_H_
 
 #include <cstdarg>
+#include <cstdint>
 #include <cstdio>
 #include <string>
 #include <syslog.h>
@@ -42,7 +43,7 @@
 class S3fsLog
 {
     public:
-        enum s3fs_log_level{
+        enum s3fs_log_level : uint8_t {
             LEVEL_CRIT = 0,          // LEVEL_CRIT
             LEVEL_ERR  = 1,          // LEVEL_ERR
             LEVEL_WARN = 3,          // LEVEL_WARNING

src/s3fs_util.h

@@ -21,7 +21,10 @@
 #ifndef S3FS_S3FS_UTIL_H_
 #define S3FS_S3FS_UTIL_H_
 
+#include <cstdint>
 #include <functional>
+#include <string>
+#include <sys/stat.h>
 
 #ifndef CLOCK_REALTIME
 #define CLOCK_REALTIME          0
@@ -61,7 +64,7 @@ void print_launch_message(int argc, char** argv);
 //
 // Utility for nanosecond time(timespec)
 //
-enum class stat_time_type{
+enum class stat_time_type : uint8_t {
     ATIME,
     MTIME,
     CTIME

src/s3fs_xml.cpp

@@ -380,7 +380,7 @@ int append_objects_from_xml_ex(const char* path, xmlDocPtr doc, xmlXPathContextP
             free(name);
 
             if(prefix){
-                head.common_prefixes.push_back(decname);
+                head.AddCommonPrefix(decname);
             }
             if(!head.insert(decname.c_str(), (!stretag.empty() ? stretag.c_str() : nullptr), is_dir)){
                 S3FS_PRN_ERR("insert_object returns with error.");

src/s3objlist.h

@@ -32,9 +32,7 @@ struct s3obj_entry{
     std::string normalname; // normalized name: if empty, object is normalized name.
     std::string orgname;    // original name: if empty, object is original name.
     std::string etag;
-    bool        is_dir;
+    bool        is_dir = false;
-
-    s3obj_entry() : is_dir(false) {}
 };
 
 typedef std::map<std::string, struct s3obj_entry> s3obj_t;
@@ -47,10 +45,8 @@ class S3ObjList
 {
     private:
         s3obj_t objects;
-    public:
         std::vector<std::string> common_prefixes;
 
-    private:
         bool insert_normalized(const char* name, const char* normalized, bool is_dir);
         const s3obj_entry* GetS3Obj(const char* name) const;
 
@@ -58,14 +54,13 @@ class S3ObjList
         s3obj_t::const_iterator end() const { return objects.end(); }
 
     public:
-        S3ObjList() {}
-        ~S3ObjList() {}
-
         bool IsEmpty() const { return objects.empty(); }
         bool insert(const char* name, const char* etag = nullptr, bool is_dir = false);
         std::string GetOrgName(const char* name) const;
         std::string GetNormalizedName(const char* name) const;
         std::string GetETag(const char* name) const;
+        const std::vector<std::string>& GetCommonPrefixes() const { return common_prefixes; }
+        void AddCommonPrefix(std::string prefix) { common_prefixes.push_back(std::move(prefix)); }
         bool IsDir(const char* name) const;
         bool GetNameList(s3obj_list_t& list, bool OnlyNormalized = true, bool CutSlash = true) const;
         bool GetLastName(std::string& lastname) const;

src/string_util.cpp

@@ -37,7 +37,7 @@
 // Functions
 //-------------------------------------------------------------------
 
-std::string str(const struct timespec value)
+std::string str(const struct timespec& value)
 {
     std::ostringstream s;
     s << value.tv_sec;

src/string_util.h

@@ -22,6 +22,7 @@
 #define S3FS_STRING_UTIL_H_
 
 #include <cstring>
+#include <ctime>
 #include <string>
 
 //
@@ -53,7 +54,7 @@ static inline const char* SAFESTRPTR(const char *strptr) { return strptr ? strpt
 // Utilities
 //-------------------------------------------------------------------
 // TODO: rename to to_string?
-std::string str(const struct timespec value);
+std::string str(const struct timespec& value);
 
 #ifdef __MSYS__
 //

src/test_util.h

@@ -21,13 +21,13 @@
 #ifndef S3FS_TEST_UTIL_H_
 #define S3FS_TEST_UTIL_H_
 
+#include <cstdio>
 #include <cstdlib>
 #include <iostream>
-#include <stdio.h>
 
 #include "string_util.h"
 
-template <typename T> void assert_equals(const T &x, const T &y, const char *file, int line)
+template <typename T> inline void assert_equals(const T &x, const T &y, const char *file, int line)
 {
     if (x != y) {
         std::cerr << x << " != " << y << " at " << file << ":" << line << std::endl;
@@ -36,7 +36,7 @@ template <typename T> void assert_equals(const T &x, const T &y, const char *fil
     }
 }
 
-template <> void assert_equals(const std::string &x, const std::string &y, const char *file, int line)
+template <> inline void assert_equals(const std::string &x, const std::string &y, const char *file, int line)
 {
     if (x != y) {
         std::cerr << x << " != " << y << " at " << file << ":" << line << std::endl;
@@ -47,7 +47,7 @@ template <> void assert_equals(const std::string &x, const std::string &y, const
 }
 
 
-template <typename T> void assert_nequals(const T &x, const T &y, const char *file, int line)
+template <typename T> inline void assert_nequals(const T &x, const T &y, const char *file, int line)
 {
     if (x == y) {
         std::cerr << x << " == " << y << " at " << file << ":" << line << std::endl;
@@ -55,7 +55,7 @@ template <typename T> void assert_nequals(const T &x, const T &y, const char *fi
     }
 }
 
-template <> void assert_nequals(const std::string &x, const std::string &y, const char *file, int line)
+template <> inline void assert_nequals(const std::string &x, const std::string &y, const char *file, int line)
 {
     if (x == y) {
         std::cerr << x << " == " << y << " at " << file << ":" << line << std::endl;
@@ -65,7 +65,7 @@ template <> void assert_nequals(const std::string &x, const std::string &y, cons
     }
 }
 
-void assert_strequals(const char *x, const char *y, const char *file, int line)
+inline void assert_strequals(const char *x, const char *y, const char *file, int line)
 {
   if(x == nullptr && y == nullptr){
       return;
@@ -76,7 +76,7 @@ void assert_strequals(const char *x, const char *y, const char *file, int line)
   }
 }
 
-void assert_bufequals(const char *x, size_t len1, const char *y, size_t len2, const char *file, int line)
+inline void assert_bufequals(const char *x, size_t len1, const char *y, size_t len2, const char *file, int line)
 {
     if(x == nullptr && y == nullptr){
         return;

src/threadpoolman.h

@@ -23,6 +23,7 @@
 
 #include <atomic>
 #include <list>
+#include <pthread.h>
 #include <vector>
 
 #include "psemaphore.h"
@@ -45,11 +46,9 @@ typedef void* (*thpoolman_worker)(void*);               // same as start_routine
 //
 struct thpoolman_param
 {
-    void*            args;
+    void*            args = nullptr;
-    Semaphore*       psem;
+    Semaphore*       psem = nullptr;
-    thpoolman_worker pfunc;
+    thpoolman_worker pfunc = nullptr;
-
-    thpoolman_param() : args(nullptr), psem(nullptr), pfunc(nullptr) {}
 };
 
 typedef std::list<thpoolman_param> thpoolman_params_t;

src/types.h

@@ -22,6 +22,7 @@
 #define S3FS_TYPES_H_
 
 #include <cstdlib>
+#include <cstdint>
 #include <cstring>
 #include <string>
 #include <map>
@@ -53,7 +54,7 @@ typedef std::map<std::string, std::string> xattrs_t;
 //-------------------------------------------------------------------
 // acl_t
 //-------------------------------------------------------------------
-enum class acl_t{
+enum class acl_t : uint8_t {
     PRIVATE,
     PUBLIC_READ,
     PUBLIC_READ_WRITE,
@@ -116,14 +117,14 @@ inline acl_t to_acl(const char *acl)
 //-------------------------------------------------------------------
 // sse_type_t
 //-------------------------------------------------------------------
-enum class sse_type_t{
+enum class sse_type_t : uint8_t {
     SSE_DISABLE = 0,      // not use server side encrypting
     SSE_S3,               // server side encrypting by S3 key
     SSE_C,                // server side encrypting by custom key
     SSE_KMS               // server side encrypting by kms id
 };
 
-enum class signature_type_t {
+enum class signature_type_t  : uint8_t {
     V2_ONLY,
     V4_ONLY,
     V2_OR_V4
@@ -184,7 +185,7 @@ struct petagpool
 //
 struct filepart
 {
-    bool         uploaded;    // does finish uploading
+    bool         uploaded = false;  // does finish uploading
     std::string  etag;        // expected etag value
     int          fd;          // base file(temporary full file) descriptor
     off_t        startpos;    // seek fd point for uploading
@@ -192,7 +193,7 @@ struct filepart
     bool         is_copy;     // whether is copy multipart
     etagpair*    petag;       // use only parallel upload
 
-    explicit filepart(bool is_uploaded = false, int _fd = -1, off_t part_start = 0, off_t part_size = -1, bool is_copy_part = false, etagpair* petagpair = nullptr) : uploaded(false), fd(_fd), startpos(part_start), size(part_size), is_copy(is_copy_part), petag(petagpair) {}
+    explicit filepart(bool is_uploaded = false, int _fd = -1, off_t part_start = 0, off_t part_size = -1, bool is_copy_part = false, etagpair* petagpair = nullptr) : fd(_fd), startpos(part_start), size(part_size), is_copy(is_copy_part), petag(petagpair) {}
 
     ~filepart()
     {
@@ -215,7 +216,7 @@ struct filepart
         if(-1 == partnum){
             partnum = static_cast<int>(list.size()) + 1;
         }
-        list.push_back(etagpair(nullptr, partnum));
+        list.emplace_back(nullptr, partnum);
         petag = &list.back();
     }
 
@@ -267,7 +268,7 @@ struct untreatedpart
     // Check if the areas overlap
     // However, even if the areas do not overlap, this method returns true if areas are adjacent.
     //
-    bool check_overlap(off_t chk_start, off_t chk_size)
+    bool check_overlap(off_t chk_start, off_t chk_size) const
     {
         if(chk_start < 0 || chk_size <= 0 || start < 0 || size <= 0 || (chk_start + chk_size) < start || (start + size) < chk_start){
             return false;