From 9555729fe99da37e46af28e8740be4b20dff77c3 Mon Sep 17 00:00:00 2001 From: Falk Werner Date: Fri, 6 Jan 2023 16:50:32 +0100 Subject: [PATCH] introduce clang tidy --- .github/workflows/build.yml | 2 +- CMakeLists.txt | 10 ++ src/webfuse/filesystem/empty_filesystem.cpp | 20 +-- src/webfuse/filesystem/empty_filesystem.hpp | 2 +- src/webfuse/filesystem/filemode.cpp | 4 +- src/webfuse/filesystem/status.cpp | 160 ++++++++++---------- src/webfuse/fuse.cpp | 2 +- src/webfuse/webfuse.cpp | 2 +- src/webfuse/ws/client.cpp | 4 +- src/webfuse/ws/config.cpp | 9 +- src/webfuse/ws/messagereader.cpp | 28 ++-- src/webfuse/ws/messagewriter.cpp | 15 +- src/webfuse/ws/server.cpp | 87 ++++++----- 13 files changed, 181 insertions(+), 164 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5cccfa6..030327b 100644 --- a/.github/workflows/build.yml +++ b/.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}} diff --git a/CMakeLists.txt b/CMakeLists.txt index 209bdda..586cc15 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/src/webfuse/filesystem/empty_filesystem.cpp b/src/webfuse/filesystem/empty_filesystem.cpp index 3b37e3a..e3af5b8 100644 --- a/src/webfuse/filesystem/empty_filesystem.cpp +++ b/src/webfuse/filesystem/empty_filesystem.cpp @@ -10,10 +10,8 @@ int empty_filesystem::access(std::string const & path, int mode) { return 0; } - else - { - return -ENOENT; - } + + return -ENOENT; } int empty_filesystem::getattr(std::string const & path, struct stat * attr) @@ -22,13 +20,11 @@ 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; - } + + return -ENOENT; } int empty_filesystem::readlink(std::string const & path, std::string & out) @@ -121,10 +117,8 @@ int empty_filesystem::readdir(std::string const & path, std::vector { return 0; } - else - { - return -ENOENT; - } + + return -ENOENT; } int empty_filesystem::rmdir(std::string const & path) diff --git a/src/webfuse/filesystem/empty_filesystem.hpp b/src/webfuse/filesystem/empty_filesystem.hpp index 2b7400b..16b390a 100644 --- a/src/webfuse/filesystem/empty_filesystem.hpp +++ b/src/webfuse/filesystem/empty_filesystem.hpp @@ -38,7 +38,7 @@ public: int readdir(std::string const & path, std::vector & 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; }; } diff --git a/src/webfuse/filesystem/filemode.cpp b/src/webfuse/filesystem/filemode.cpp index 6d1213f..14d54e8 100644 --- a/src/webfuse/filesystem/filemode.cpp +++ b/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; } diff --git a/src/webfuse/filesystem/status.cpp b/src/webfuse/filesystem/status.cpp index b3b3977..9864380 100644 --- a/src/webfuse/filesystem/status.cpp +++ b/src/webfuse/filesystem/status.cpp @@ -21,48 +21,46 @@ status status::from_fusestatus(int value) { return static_cast(value); } - else + + switch(value) { - switch(value) - { - case -E2BIG: return status::bad_e2big; - case -EACCES: return status::bad_eacces; - case -EAGAIN: return status::bad_eagain; - case -EBADF: return status::bad_ebadf; - case -EBUSY: return status::bad_ebusy; - case -EDESTADDRREQ: return status::bad_edestaddrreq; - case -EDQUOT: return status::bad_edquot; - case -EEXIST: return status::bad_eexist; - case -EFAULT: return status::bad_efault; - case -EFBIG: return status::bad_efbig; - case -EINTR: return status::bad_eintr; - case -EINVAL: return status::bad_einval; - case -EIO: return status::bad_eio; - case -EISDIR: return status::bad_eisdir; - case -ELOOP: return status::bad_eloop; - case -EMFILE: return status::bad_emfile; - case -EMLINK: return status::bad_emlink; - case -ENAMETOOLONG: return status::bad_enametoolong; - case -ENFILE: return status::bad_enfile; - case -ENODATA: return status::bad_enodata; - case -ENODEV: return status::bad_enodev; - case -ENOENT: return status::bad_enoent; - case -ENOMEM: return status::bad_enomem; - case -ENOSPC: return status::bad_enospc; - case -ENOSYS: return status::bad_enosys; - case -ENOTDIR: return status::bad_enotdir; - case -ENOTEMPTY: return status::bad_enotempty; - case -ENOTSUP: return status::bad_enotsup; - case -ENXIO: return status::bad_enxio; - case -EOVERFLOW: return status::bad_eoverflow; - case -EPERM: return status ::bad_eperm; - case -EPIPE: return status::bad_epipe; - case -ERANGE: return status::bad_erange; - case -EROFS: return status::bad_erofs; - case -ETXTBSY: return status::bad_etxtbsy; - case -EXDEV: return status::bad_exdev; - default: return static_cast(value); - } + case -E2BIG: return status::bad_e2big; + case -EACCES: return status::bad_eacces; + case -EAGAIN: return status::bad_eagain; + case -EBADF: return status::bad_ebadf; + case -EBUSY: return status::bad_ebusy; + case -EDESTADDRREQ: return status::bad_edestaddrreq; + case -EDQUOT: return status::bad_edquot; + case -EEXIST: return status::bad_eexist; + case -EFAULT: return status::bad_efault; + case -EFBIG: return status::bad_efbig; + case -EINTR: return status::bad_eintr; + case -EINVAL: return status::bad_einval; + case -EIO: return status::bad_eio; + case -EISDIR: return status::bad_eisdir; + case -ELOOP: return status::bad_eloop; + case -EMFILE: return status::bad_emfile; + case -EMLINK: return status::bad_emlink; + case -ENAMETOOLONG: return status::bad_enametoolong; + case -ENFILE: return status::bad_enfile; + case -ENODATA: return status::bad_enodata; + case -ENODEV: return status::bad_enodev; + case -ENOENT: return status::bad_enoent; + case -ENOMEM: return status::bad_enomem; + case -ENOSPC: return status::bad_enospc; + case -ENOSYS: return status::bad_enosys; + case -ENOTDIR: return status::bad_enotdir; + case -ENOTEMPTY: return status::bad_enotempty; + case -ENOTSUP: return status::bad_enotsup; + case -ENXIO: return status::bad_enxio; + case -EOVERFLOW: return status::bad_eoverflow; + case -EPERM: return status ::bad_eperm; + case -EPIPE: return status::bad_epipe; + case -ERANGE: return status::bad_erange; + case -EROFS: return status::bad_erofs; + case -ETXTBSY: return status::bad_etxtbsy; + case -EXDEV: return status::bad_exdev; + default: return static_cast(value); } } @@ -72,48 +70,46 @@ int status::to_fusestatus() const { return static_cast(value_); } - else + + switch(value_) { - switch(value_) - { - case status::bad_e2big: return -E2BIG; - case status::bad_eacces: return -EACCES; - case status::bad_eagain: return -EAGAIN; - case status::bad_ebadf: return -EBADF; - case status::bad_ebusy: return -EBUSY; - case status::bad_edestaddrreq: return -EDESTADDRREQ; - case status::bad_edquot: return -EDQUOT; - case status::bad_eexist: return -EEXIST; - case status::bad_efault: return -EFAULT; - case status::bad_efbig: return -EFBIG; - case status::bad_eintr: return -EINTR; - case status::bad_einval: return -EINVAL; - case status::bad_eio: return -EIO; - case status::bad_eisdir: return -EISDIR; - case status::bad_eloop: return -ELOOP; - case status::bad_emfile: return -EMFILE; - case status::bad_emlink: return -EMLINK; - case status::bad_enametoolong: return -ENAMETOOLONG; - case status::bad_enfile: return -ENFILE; - case status::bad_enodata: return -ENODATA; - case status::bad_enodev: return -ENODEV; - case status::bad_enoent: return -ENOENT; - case status::bad_enomem: return -ENOMEM; - case status::bad_enospc: return -ENOSPC; - case status::bad_enosys: return -ENOSYS; - case status::bad_enotdir: return -ENOTDIR; - case status::bad_enotempty: return -ENOTEMPTY; - case status::bad_enotsup: return -ENOTSUP; - case status::bad_enxio: return -ENXIO; - case status::bad_eoverflow: return -EOVERFLOW; - case status::bad_eperm: return -EPERM; - case status::bad_epipe: return -EPIPE; - case status::bad_erange: return -ERANGE; - case status::bad_erofs: return -EROFS; - case status::bad_etxtbsy: return -ETXTBSY; - case status::bad_exdev: return -EXDEV; - default: return static_cast(value_); - } + case status::bad_e2big: return -E2BIG; + case status::bad_eacces: return -EACCES; + case status::bad_eagain: return -EAGAIN; + case status::bad_ebadf: return -EBADF; + case status::bad_ebusy: return -EBUSY; + case status::bad_edestaddrreq: return -EDESTADDRREQ; + case status::bad_edquot: return -EDQUOT; + case status::bad_eexist: return -EEXIST; + case status::bad_efault: return -EFAULT; + case status::bad_efbig: return -EFBIG; + case status::bad_eintr: return -EINTR; + case status::bad_einval: return -EINVAL; + case status::bad_eio: return -EIO; + case status::bad_eisdir: return -EISDIR; + case status::bad_eloop: return -ELOOP; + case status::bad_emfile: return -EMFILE; + case status::bad_emlink: return -EMLINK; + case status::bad_enametoolong: return -ENAMETOOLONG; + case status::bad_enfile: return -ENFILE; + case status::bad_enodata: return -ENODATA; + case status::bad_enodev: return -ENODEV; + case status::bad_enoent: return -ENOENT; + case status::bad_enomem: return -ENOMEM; + case status::bad_enospc: return -ENOSPC; + case status::bad_enosys: return -ENOSYS; + case status::bad_enotdir: return -ENOTDIR; + case status::bad_enotempty: return -ENOTEMPTY; + case status::bad_enotsup: return -ENOTSUP; + case status::bad_enxio: return -ENXIO; + case status::bad_eoverflow: return -EOVERFLOW; + case status::bad_eperm: return -EPERM; + case status::bad_epipe: return -EPIPE; + case status::bad_erange: return -ERANGE; + case status::bad_erofs: return -EROFS; + case status::bad_etxtbsy: return -ETXTBSY; + case status::bad_exdev: return -EXDEV; + default: return static_cast(value_); } } diff --git a/src/webfuse/fuse.cpp b/src/webfuse/fuse.cpp index 8600d02..4132025 100644 --- a/src/webfuse/fuse.cpp +++ b/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); diff --git a/src/webfuse/webfuse.cpp b/src/webfuse/webfuse.cpp index e06a6ee..3977692 100644 --- a/src/webfuse/webfuse.cpp +++ b/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); diff --git a/src/webfuse/ws/client.cpp b/src/webfuse/ws/client.cpp index d47fdb4..e164362 100644 --- a/src/webfuse/ws/client.cpp +++ b/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(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(&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 = "/"; diff --git a/src/webfuse/ws/config.cpp b/src/webfuse/ws/config.cpp index 4acbbe6..f7a68e1 100644 --- a/src/webfuse/ws/config.cpp +++ b/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) { } diff --git a/src/webfuse/ws/messagereader.cpp b/src/webfuse/ws/messagereader.cpp index 1f966e3..4c261ad 100644 --- a/src/webfuse/ws/messagereader.cpp +++ b/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"); - } + + 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 - { - throw std::runtime_error("out of bounds"); - } + // 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,10 +137,9 @@ uint64_t messagereader::read_u64() pos += 8; return value; } - else - { - throw std::runtime_error("out of bounds"); - } + // NOLINTEND(readability-magic-numbers) + + throw std::runtime_error("out of bounds"); } @@ -165,10 +163,8 @@ std::string messagereader::read_bytes() pos += size; return std::move(value); } - else - { - throw std::runtime_error("out of bounds"); - } + + throw std::runtime_error("out of bounds"); } void messagereader::read_strings(std::vector &entries) diff --git a/src/webfuse/ws/messagewriter.cpp b/src/webfuse/ws/messagewriter.cpp index 59f5d01..fe565ed 100644 --- a/src/webfuse/ws/messagewriter.cpp +++ b/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(); + 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(&data.data()[offset]); + void * to = reinterpret_cast(&data[offset]); void const * from = reinterpret_cast(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(&data.data()[LWS_PRE]); + void * result = reinterpret_cast(&data[LWS_PRE]); return reinterpret_cast(result); } diff --git a/src/webfuse/ws/server.cpp b/src/webfuse/ws/server.cpp index aa9ad67..8dde316 100644 --- a/src/webfuse/ws/server.cpp +++ b/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> pending_responses; }; + +void do_receive(void * in, int len, lws* wsi, user_data * data) +{ + auto * fragment = reinterpret_cast(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 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: - { - auto * fragment = reinterpret_cast(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; - } - - - } - } + do_receive(in, len, wsi, data); 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 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 lock(data.mut); if (!data.requests.empty()) { if (nullptr != data.connection) @@ -264,7 +269,7 @@ messagereader ws_server::perform(messagewriter writer) std::promise p; f = p.get_future(); - std::lock_guard lock(d->data.mut); + std::lock_guard 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"); }