introduce clang tidy

.github/workflows/build.yml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v3
       
       - name: Install APT dependencies
-        run: sudo apt install libfuse3-dev libwebsockets-dev libgtest-dev libgmock-dev valgrind
+        run: sudo apt install libfuse3-dev libwebsockets-dev libgtest-dev libgmock-dev clang-tidy valgrind
       
       - name: Configure CMake
         run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}

CMakeLists.txt

@@ -1,6 +1,9 @@
 cmake_minimum_required(VERSION 3.10)
 project(webfuse VERSION 2.0.0)
 
+option(WITHOUT_TEST "Disables unit and integration tests" OFF)
+option(WITHOUT_CLANG_TIDY "Disables clang tidy" OFF)
+
 set (CMAKE_CXX_STANDARD 17)
 
 find_package(PkgConfig REQUIRED)
@@ -33,6 +36,13 @@ add_library(webfuse_static STATIC
 target_include_directories(webfuse_static PUBLIC src)
 target_link_libraries(webfuse_static PUBLIC PkgConfig::FUSE PkgConfig::LWS)
 
+if(NOT(WITHOUT_CLANG_TIDY))
+set_property(
+    TARGET webfuse_static
+    PROPERTY CXX_CLANG_TIDY clang-tidy -checks=-*,readability-*,-readability-identifier-length -warnings-as-errors=*)
+endif()
+
+
 add_executable(webfuse
     src/main.cpp)
 

src/webfuse/filesystem/empty_filesystem.cpp

@@ -10,11 +10,9 @@ int empty_filesystem::access(std::string const & path, int mode)
     {
         return 0;
     }
-    else
+
-    {
     return -ENOENT;
 }
-}
 
 int empty_filesystem::getattr(std::string const & path, struct stat * attr)
 {
@@ -22,14 +20,12 @@ int empty_filesystem::getattr(std::string const & path, struct stat * attr)
     {
         attr->st_ino = 1;
         attr->st_nlink = 1;
-        attr->st_mode = S_IFDIR | 0555;
+        attr->st_mode = S_IFDIR | 0555; // NOLINT(readability-magic-numbers)
         return 0;
     }
-    else
+
-    {
     return -ENOENT;
 }
-}
 
 int empty_filesystem::readlink(std::string const & path, std::string & out)
 {
@@ -121,11 +117,9 @@ int empty_filesystem::readdir(std::string const & path, std::vector<std::string>
     {
         return 0;
     }
-    else
+
-    {
     return -ENOENT;
 }
-}
 
 int empty_filesystem::rmdir(std::string const & path)
 {

src/webfuse/filesystem/empty_filesystem.hpp

@@ -38,7 +38,7 @@ public:
     int readdir(std::string const & path, std::vector<std::string> & entries) override;
     int rmdir(std::string const & path) override;
 
-    int statfs(std::string const & path, struct statvfs * statistivs) override;
+    int statfs(std::string const & path, struct statvfs * statistics) override;
 };
 
 }

src/webfuse/filesystem/filemode.cpp

@@ -18,7 +18,7 @@ filemode::operator uint32_t() const
 
 filemode filemode::from_mode(mode_t value)
 {
-    uint32_t result = value & 07777;
+    uint32_t result = value & 07777; // NOLINT(readability-magic-numbers)
 
     if (S_ISREG(value) ) { result |= filemode::reg;  }
     if (S_ISDIR(value) ) { result |= filemode::dir;  }
@@ -33,7 +33,7 @@ filemode filemode::from_mode(mode_t value)
 
 mode_t filemode::to_mode() const
 {
-    mode_t result = value_ & 07777;
+    mode_t result = value_ & 07777; // NOLINT(readability-magic-numbers)
 
     if (is_reg() ) { result |= S_IFREG; }
     if (is_dir() ) { result |= S_IFDIR; }

src/webfuse/filesystem/status.cpp

@@ -21,8 +21,7 @@ status status::from_fusestatus(int value)
     {
         return static_cast<int32_t>(value);
     }
-    else
+
-    {
     switch(value)
     {
         case -E2BIG: return status::bad_e2big;
@@ -64,7 +63,6 @@ status status::from_fusestatus(int value)
         default: return static_cast<int32_t>(value);
     }
 }
-}
 
 int status::to_fusestatus() const
 {
@@ -72,8 +70,7 @@ int status::to_fusestatus() const
     {
         return static_cast<int>(value_);
     }
-    else
+
-    {
     switch(value_)
     {
         case status::bad_e2big: return -E2BIG;
@@ -115,7 +112,6 @@ int status::to_fusestatus() const
         default: return static_cast<int32_t>(value_);
     }
 }
-}
 
 bool status::is_good() const
 {

src/webfuse/fuse.cpp

@@ -106,7 +106,7 @@ static int fs_truncate(char const * path, off_t size, fuse_file_info * info)
 static int fs_fsync(char const * path, int isdatasync, fuse_file_info * info)
 {
     auto * const fs = fs_get_filesystem();
-    bool const is_datasync = (is_datasync != 0);
+    bool const is_datasync = (isdatasync != 0);
     auto const handle = fs_get_handle(info);
 
     return fs->fsync(path, is_datasync, handle);

src/webfuse/webfuse.cpp

@@ -6,7 +6,7 @@
 namespace webfuse
 {
 
-int app::run(int argc, char * argv[])
+int app::run(int argc, char * argv[]) // NOLINT(readability-convert-member-functions-to-static)
 {
     ws_config config;
     ws_server server(config);

src/webfuse/ws/client.cpp

@@ -48,7 +48,7 @@ extern "C" int webfuse_client_callback(lws * wsi, lws_callback_reasons reason, v
                 {
                     auto * fragment = reinterpret_cast<char*>(in);
                     context->current_message.append(fragment, length);
-                    if (lws_is_final_fragment(wsi))
+                    if (0 != lws_is_final_fragment(wsi))
                     {
                         try
                         {
@@ -137,7 +137,7 @@ public:
         lws_client_connect_info info;
         memset(reinterpret_cast<void*>(&info), 0, sizeof(lws_client_connect_info));
         info.context = context;
-        info.port = 8081;
+        info.port = 8081;  //NOLINT(readability-magic-numbers)
         info.address = "localhost";
         info.host = "localhost";
         info.path = "/";

src/webfuse/ws/config.cpp

@@ -1,10 +1,17 @@
 #include "webfuse/ws/config.hpp"
 
+namespace
+{
+
+constexpr int const default_port = 8081;
+
+}
+
 namespace webfuse
 {
 
 ws_config::ws_config()
-: port(8081)
+: port(default_port)
 {
 
 }

src/webfuse/ws/messagereader.cpp

@@ -98,14 +98,13 @@ uint8_t messagereader::read_u8()
         pos++;
         return value;
     }
-    else
+
-    {
     throw std::runtime_error("out of bounds");
 }
-}
 
 uint32_t messagereader::read_u32()
 {
+    // NOLINTBEGIN(readability-magic-numbers)
     if ((pos + 3) < data.size())
     {
         uint32_t value = 
@@ -116,14 +115,14 @@ uint32_t messagereader::read_u32()
         pos += 4;
         return value;
     }
-    else
+    // NOLINTEND(readability-magic-numbers)
-    {
+
     throw std::runtime_error("out of bounds");
 }
-}
 
 uint64_t messagereader::read_u64()
 {
+    // NOLINTBEGIN(readability-magic-numbers)
     if ((pos + 7) < data.size())
     {
         uint32_t value = 
@@ -138,11 +137,10 @@ uint64_t messagereader::read_u64()
         pos += 8;
         return value;
     }
-    else
+    // NOLINTEND(readability-magic-numbers)
-    {
+
     throw std::runtime_error("out of bounds");
 }
-}
 
 
 int32_t messagereader::read_i32()
@@ -165,11 +163,9 @@ std::string messagereader::read_bytes()
         pos += size;
         return std::move(value);
     }
-    else
+
-    {
     throw std::runtime_error("out of bounds");
 }
-}
 
 void messagereader::read_strings(std::vector<std::string> &entries)
 {

src/webfuse/ws/messagewriter.cpp

@@ -47,10 +47,13 @@ messagewriter& messagewriter::operator=(messagewriter && other)
 void messagewriter::set_id(uint32_t value)
 {
     id = value;
+
+    // NOLINTBEGIN(readability-magic-numbers)
     data[LWS_PRE    ] = (id >> 24) & 0xff;
     data[LWS_PRE + 1] = (id >> 16) & 0xff;
     data[LWS_PRE + 2] = (id >>  8) & 0xff;
     data[LWS_PRE + 3] =  id        & 0xff;
+    // NOLINTEND(readability-magic-numbers)
 }
 
 uint32_t messagewriter::get_id() const
@@ -81,16 +84,21 @@ void messagewriter::write_i32(int32_t value)
 void messagewriter::write_u32(uint32_t value)
 {
     auto const offset = data.size(); 
+
+    // NOLINTBEGIN(readability-magic-numbers)
     data.resize(offset + 4);
     data[offset    ] = (value >> 24) & 0xff;
     data[offset + 1] = (value >> 16) & 0xff;
     data[offset + 2] = (value >>  8) & 0xff;
     data[offset + 3] =  value        & 0xff;
+    // NOLINTEND(readability-magic-numbers)
 }
 
 void messagewriter::write_u64(uint64_t value)
 {
     auto const offset = data.size();
+
+    // NOLINTBEGIN(readability-magic-numbers)
     data.resize(offset + 8);
     data[offset    ] = (value >> 56) & 0xff;
     data[offset + 1] = (value >> 48) & 0xff;
@@ -100,6 +108,7 @@ void messagewriter::write_u64(uint64_t value)
     data[offset + 5] = (value >> 16) & 0xff;
     data[offset + 6] = (value >>  8) & 0xff;
     data[offset + 7] =  value        & 0xff;
+    // NOLINTEND(readability-magic-numbers)
 }
 
 void messagewriter::write_str(std::string const &value)
@@ -116,7 +125,7 @@ void messagewriter::write_data(char const * buffer, size_t size)
     {
         auto const offset = data.size();
         data.resize(offset + effective_size);
-        void * to = reinterpret_cast<void*>(&data.data()[offset]);
+        void * to = reinterpret_cast<void*>(&data[offset]);
         void const * from = reinterpret_cast<void const *>(buffer);
         memcpy(to, from, effective_size);
     }
@@ -205,7 +214,7 @@ void messagewriter::write_statistics(struct statvfs const * statistics)
 unsigned char * messagewriter::get_data(size_t &size)
 {
     size = data.size() - LWS_PRE;
-    void * result = reinterpret_cast<void *>(&data.data()[LWS_PRE]);
+    void * result = reinterpret_cast<void *>(&data[LWS_PRE]);
 
     return reinterpret_cast<unsigned char *>(result);
 }

src/webfuse/ws/server.cpp

@@ -21,6 +21,8 @@
 namespace
 {
 
+constexpr int64_t const timeout_secs = 10;
+
 struct user_data
 {
     struct lws * connection = nullptr;
@@ -32,6 +34,45 @@ struct user_data
     std::unordered_map<uint32_t, std::promise<webfuse::messagereader>> pending_responses;
 };
 
+
+void do_receive(void * in, int len, lws* wsi, user_data * data)
+{
+    auto * fragment = reinterpret_cast<char*>(in);
+    data->current_message.append(fragment, len);
+    if (0 != lws_is_final_fragment(wsi))
+    {
+        try
+        {
+            webfuse::messagereader reader(data->current_message);
+            uint32_t id = reader.read_u32();
+            uint8_t message_type = reader.read_u8();
+
+            std::lock_guard<std::mutex> lock(data->mut);
+            auto it = data->pending_responses.find(id);
+            if (it != data->pending_responses.end())
+            {
+                it->second.set_value(std::move(reader));
+                data->pending_responses.erase(it);
+            }
+            else
+            {
+                // ToDo: log request not found
+                std::cout << "warning: request not found: id=" << id << std::endl;
+                for(auto const & entry: data->pending_responses)
+                {
+                    std::cout << "\t" << entry.first << std::endl;
+                }
+            }
+        }
+        catch(...)
+        {
+            // ToDo: log invalid message
+            std::cout << "warning: invalid message" << std::endl;
+        }
+    }
+}
+
+
 }
 
 extern "C"
@@ -66,43 +107,7 @@ static int ws_server_callback(struct lws *wsi, enum lws_callback_reasons reason,
             }
             break;
         case LWS_CALLBACK_RECEIVE:
-            {
+            do_receive(in, len, wsi, data);
-                auto * fragment = reinterpret_cast<char*>(in);
-                data->current_message.append(fragment, len);
-                if (lws_is_final_fragment(wsi))
-                {
-                    try
-                    {
-                        webfuse::messagereader reader(data->current_message);
-                        uint32_t id = reader.read_u32();
-                        uint8_t message_type = reader.read_u8();
-
-                        std::lock_guard lock(data->mut);
-                        auto it = data->pending_responses.find(id);
-                        if (it != data->pending_responses.end())
-                        {
-                            it->second.set_value(std::move(reader));
-                            data->pending_responses.erase(it);
-                        }
-                        else
-                        {
-                            // ToDo: log request not found
-                            std::cout << "warning: request not found: id=" << id << std::endl;
-                            for(auto const & entry: data->pending_responses)
-                            {
-                                std::cout << "\t" << entry.first << std::endl;
-                            }
-                        }
-                    }
-                    catch(...)
-                    {
-                        // ToDo: log invalid message
-                        std::cout << "warning: invalid message" << std::endl;
-                    }
-                    
-
-                }
-            }
             break;
         case LWS_CALLBACK_SERVER_WRITEABLE:
             {
@@ -111,7 +116,7 @@ static int ws_server_callback(struct lws *wsi, enum lws_callback_reasons reason,
                 bool has_more = false;
 
                 {
-                    std::lock_guard lock(data->mut);
+                    std::lock_guard<std::mutex> lock(data->mut);
                     has_msg = !(data->requests.empty());
                     if (has_msg)
                     {
@@ -181,7 +186,7 @@ public:
             while (!shutdown_requested)
             {
                 {
-                    std::lock_guard lock(data.mut);
+                    std::lock_guard<std::mutex> lock(data.mut);
                     if (!data.requests.empty())
                     {
                         if (nullptr != data.connection)
@@ -264,7 +269,7 @@ messagereader ws_server::perform(messagewriter writer)
         std::promise<messagereader> p;
         f = p.get_future();
 
-        std::lock_guard lock(d->data.mut);
+        std::lock_guard<std::mutex> lock(d->data.mut);
         uint32_t id = d->next_id();
         writer.set_id(id);
         d->data.requests.emplace(std::move(writer));
@@ -272,7 +277,7 @@ messagereader ws_server::perform(messagewriter writer)
     }
 
     lws_cancel_service(d->context);
-    if(std::future_status::timeout == f.wait_for(std::chrono::seconds(10)))
+    if(std::future_status::timeout == f.wait_for(std::chrono::seconds(timeout_secs)))
     {
         throw std::runtime_error("timeout");
     }