From: Joel Rosdahl Date: Sat, 19 Apr 2025 18:30:01 +0000 (+0200) Subject: chore: Fix some Clang-Tidy warnings X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ec79e3e85cca94be2e642c90214d20a5ff092ff1;p=thirdparty%2Fccache.git chore: Fix some Clang-Tidy warnings --- diff --git a/cmake/GenerateConfigurationFile.cmake b/cmake/GenerateConfigurationFile.cmake index bf2cd5a5..fcc7f025 100644 --- a/cmake/GenerateConfigurationFile.cmake +++ b/cmake/GenerateConfigurationFile.cmake @@ -25,7 +25,6 @@ endforeach() include(CheckFunctionExists) set(functions - asctime_r getopt_long getpwuid localtime_r diff --git a/cmake/config.h.in b/cmake/config.h.in index 5e6387ed..f7228d53 100644 --- a/cmake/config.h.in +++ b/cmake/config.h.in @@ -113,9 +113,6 @@ // === Functions === -// Define if you have the "asctime_r" function. -#cmakedefine HAVE_ASCTIME_R - // Define if you have the "getopt_long" function. #cmakedefine HAVE_GETOPT_LONG diff --git a/src/ccache/.clang-tidy b/src/ccache/.clang-tidy index b82182ea..f7dc1c05 100644 --- a/src/ccache/.clang-tidy +++ b/src/ccache/.clang-tidy @@ -16,6 +16,7 @@ Checks: ' -bugprone-implicit-widening-of-multiplication-result, -bugprone-narrowing-conversions, -bugprone-signed-char-misuse, + -bugprone-switch-missing-default-case,-warnings-as-errors, -bugprone-unhandled-exception-at-new, cert-*, -cert-dcl50-cpp, @@ -59,12 +60,14 @@ Checks: ' performance-*, -performance-unnecessary-value-param, readability-*, + -readability-avoid-nested-conditional-operator, -readability-convert-member-functions-to-static, -readability-else-after-return, -readability-function-cognitive-complexity, -readability-identifier-length, -readability-implicit-bool-conversion, -readability-magic-numbers, + -readability-math-missing-parentheses, -readability-named-parameter, -readability-qualified-auto, -readability-redundant-declaration, diff --git a/src/ccache/ccache.cpp b/src/ccache/ccache.cpp index 65ed0c7d..00712987 100644 --- a/src/ccache/ccache.cpp +++ b/src/ccache/ccache.cpp @@ -221,12 +221,13 @@ prepare_debug_path(const fs::path& cwd, char timestamp[100]; const auto tm = util::localtime(time_of_invocation); if (tm) { - strftime(timestamp, sizeof(timestamp), "%Y%m%d_%H%M%S", &*tm); + (void)strftime(timestamp, sizeof(timestamp), "%Y%m%d_%H%M%S", &*tm); } else { - snprintf(timestamp, - sizeof(timestamp), - "%llu", - static_cast(time_of_invocation.sec())); + (void)snprintf( + timestamp, + sizeof(timestamp), + "%llu", + static_cast(time_of_invocation.sec())); } return FMT("{}.{}_{:06}.ccache-{}", prefix, @@ -495,18 +496,19 @@ print_included_files(const Context& ctx, FILE* fp) static tl::expected process_preprocessed_file(Context& ctx, Hash& hash, const fs::path& path) { - auto data = util::read_file(path); - if (!data) { - LOG("Failed to read {}: {}", path, data.error()); + auto content = util::read_file(path); + if (!content) { + LOG("Failed to read {}: {}", path, content.error()); return tl::unexpected(Statistic::internal_error); } std::unordered_map relative_inc_path_cache; // Bytes between p and q are pending to be hashed. - char* q = &(*data)[0]; + std::string& data = *content; + char* q = data.data(); const char* p = q; - const char* end = p + data->length(); + const char* end = p + data.length(); // There must be at least 7 characters (# 1 "x") left to potentially find an // include file path. @@ -549,7 +551,7 @@ process_preprocessed_file(Context& ctx, Hash& hash, const fs::path& path) // HP/AIX: || (q[1] == 'l' && q[2] == 'i' && q[3] == 'n' && q[4] == 'e' && q[5] == ' ')) - && (q == data->data() || q[-1] == '\n')) { + && (q == data.data() || q[-1] == '\n')) { // Workarounds for preprocessor linemarker bugs in GCC version 6. if (q[2] == '3') { if (util::starts_with(q, hash_31_command_line_newline)) { @@ -650,7 +652,7 @@ process_preprocessed_file(Context& ctx, Hash& hash, const fs::path& path) "bin directive in source code"); return tl::unexpected(Failure(Statistic::unsupported_code_directive)); } else if (strncmp(q, "___________", 10) == 0 - && (q == data->data() || q[-1] == '\n')) { + && (q == data.data() || q[-1] == '\n')) { // Unfortunately the distcc-pump wrapper outputs standard output lines: // __________Using distcc-pump from /usr/bin // __________Using # distcc servers in pump mode @@ -1093,16 +1095,17 @@ rewrite_stdout_from_compiler(const Context& ctx, util::Bytes&& stdout_data) } // The MSVC /FC option causes paths in diagnostics messages to become // absolute. Those within basedir need to be changed into relative paths. - else if (std::size_t path_end = 0; - ctx.config.compiler_type() == CompilerType::msvc - && !ctx.config.base_dir().empty() - && (path_end = core::get_diagnostics_path_length(line)) != 0) { - std::string_view abs_path = line.substr(0, path_end); - fs::path rel_path = core::make_relative_path(ctx, abs_path); - std::string line_with_rel = - util::replace_all(line, abs_path, util::pstr(rel_path).str()); - new_stdout_data.insert( - new_stdout_data.end(), line_with_rel.data(), line_with_rel.size()); + else if (ctx.config.compiler_type() == CompilerType::msvc + && !ctx.config.base_dir().empty()) { + size_t path_end = core::get_diagnostics_path_length(line); + if (path_end != 0) { + std::string_view abs_path = line.substr(0, path_end); + fs::path rel_path = core::make_relative_path(ctx, abs_path); + std::string line_with_rel = + util::replace_all(line, abs_path, util::pstr(rel_path).str()); + new_stdout_data.insert( + new_stdout_data.end(), line_with_rel.data(), line_with_rel.size()); + } } else { new_stdout_data.insert(new_stdout_data.end(), line.data(), line.size()); } @@ -2145,7 +2148,7 @@ get_result_key_from_manifest(Context& ctx, const Hash::Digest& manifest_key) std::optional result_key; size_t read_manifests = 0; ctx.storage.get( - manifest_key, core::CacheEntryType::manifest, [&](util::Bytes&& value) { + manifest_key, core::CacheEntryType::manifest, [&](const auto& value) { try { read_manifest(ctx, value); ++read_manifests; @@ -2284,7 +2287,7 @@ calculate_result_and_manifest_key(Context& ctx, return std::make_pair(result_key, manifest_key); } -enum class FromCacheCallMode { direct, cpp }; +enum class FromCacheCallMode : uint8_t { direct, cpp }; // Try to return the compile result from cache. static tl::expected @@ -2567,8 +2570,9 @@ cache_compilation(int argc, const char* const* argv) } if (!result) { - if (result.error().exit_code()) { - return *result.error().exit_code(); + const auto& exit_code = result.error().exit_code(); + if (exit_code) { + return *exit_code; } // Else: Fall back to running the real compiler. fall_back_to_original_compiler = true; diff --git a/src/ccache/config.cpp b/src/ccache/config.cpp index ef0fb2a0..963202ef 100644 --- a/src/ccache/config.cpp +++ b/src/ccache/config.cpp @@ -72,7 +72,7 @@ using util::make_path; namespace { -enum class ConfigItem { +enum class ConfigItem : uint8_t { absolute_paths_in_stderr, base_dir, cache_dir, @@ -120,7 +120,7 @@ enum class ConfigItem { umask, }; -enum class ConfigKeyType { normal, alias }; +enum class ConfigKeyType : uint8_t { normal, alias }; struct ConfigKeyTableEntry { @@ -1217,7 +1217,7 @@ Config::set_item(const std::string& key, break; } - const std::string canonical_key = it->second.alias ? *it->second.alias : key; + const std::string& canonical_key = it->second.alias.value_or(key); const auto& [element, inserted] = m_origins.emplace(canonical_key, origin); if (!inserted) { element->second = origin; diff --git a/src/ccache/core/mainoptions.cpp b/src/ccache/core/mainoptions.cpp index c3e753aa..b3119082 100644 --- a/src/ccache/core/mainoptions.cpp +++ b/src/ccache/core/mainoptions.cpp @@ -433,7 +433,7 @@ get_usage_text(const std::string_view ccache_name) return FMT(USAGE_TEXT, ccache_name); } -enum { +enum : uint8_t { CHECKSUM_FILE, CONFIG_PATH, DUMP_MANIFEST, diff --git a/src/ccache/core/manifest.cpp b/src/ccache/core/manifest.cpp index 9a0f9239..cbd2dd33 100644 --- a/src/ccache/core/manifest.cpp +++ b/src/ccache/core/manifest.cpp @@ -102,6 +102,7 @@ Manifest::read(nonstd::span data) } const auto file_count = reader.read_int(); + files.reserve(file_count); for (uint32_t i = 0; i < file_count; ++i) { files.emplace_back(reader.read_str(reader.read_int())); } diff --git a/src/ccache/core/statistics.cpp b/src/ccache/core/statistics.cpp index 2eb786dc..9d9aa3e4 100644 --- a/src/ccache/core/statistics.cpp +++ b/src/ccache/core/statistics.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Joel Rosdahl and other contributors +// Copyright (C) 2021-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -274,7 +274,7 @@ format_timestamp(const util::TimePoint& value) const auto tm = util::localtime(value); char buffer[100] = "?"; if (tm) { - strftime(buffer, sizeof(buffer), "%c", &*tm); + (void)strftime(buffer, sizeof(buffer), "%c", &*tm); } return buffer; } diff --git a/src/ccache/hashutil.cpp b/src/ccache/hashutil.cpp index c8bed7e5..1a8d4583 100644 --- a/src/ccache/hashutil.cpp +++ b/src/ccache/hashutil.cpp @@ -307,17 +307,8 @@ hash_source_code_file(const Context& ctx, return result; } hash.hash_delimiter("timestamp"); -#ifdef HAVE_ASCTIME_R - char buffer[26]; - const char* timestamp = asctime_r(&*modified_time, buffer); -#else - // cppcheck-suppress asctimeCalled; thread-safety not needed here - const char* timestamp = asctime(&*modified_time); -#endif - if (!timestamp) { - result.insert(HashSourceCode::error); - return result; - } + char timestamp[26]; + (void)strftime(timestamp, sizeof(timestamp), "%c", &*modified_time); hash.hash(timestamp); } @@ -463,8 +454,7 @@ hash_command_output(Hash& hash, pid_t pid; extern char** environ; - int result = posix_spawnp( - &pid, argv[0], &fa, nullptr, const_cast(argv), environ); + int result = posix_spawnp(&pid, argv[0], &fa, nullptr, argv, environ); posix_spawn_file_actions_destroy(&fa); close(pipefd[1]); diff --git a/src/ccache/storage/local/localstorage.cpp b/src/ccache/storage/local/localstorage.cpp index 4b13a4a3..29692b97 100644 --- a/src/ccache/storage/local/localstorage.cpp +++ b/src/ccache/storage/local/localstorage.cpp @@ -786,17 +786,17 @@ LocalStorage::evict(const ProgressReceiver& progress_receiver, std::optional max_age, std::optional namespace_) { - return do_clean_all(progress_receiver, 0, 0, max_age, namespace_); + do_clean_all(progress_receiver, 0, 0, max_age, namespace_); } void LocalStorage::clean_all(const ProgressReceiver& progress_receiver) { - return do_clean_all(progress_receiver, - m_config.max_size(), - m_config.max_files(), - std::nullopt, - std::nullopt); + do_clean_all(progress_receiver, + m_config.max_size(), + m_config.max_files(), + std::nullopt, + std::nullopt); } // Wipe all cached files in all subdirectories. @@ -1082,7 +1082,7 @@ LocalStorage::move_to_wanted_cache_level(const StatisticsCounters& counters, // to rename is OK. LOG("Moving {} to {}", cache_file_path, wanted_path); fs::rename(cache_file_path, wanted_path); - for (auto [file_number, dest_path] : m_added_raw_files) { + for (const auto& [file_number, dest_path] : m_added_raw_files) { fs::rename(dest_path, get_raw_file_path(wanted_path, file_number)); } } diff --git a/src/ccache/storage/remote/filestorage.cpp b/src/ccache/storage/remote/filestorage.cpp index 643e718b..580871cd 100644 --- a/src/ccache/storage/remote/filestorage.cpp +++ b/src/ccache/storage/remote/filestorage.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Joel Rosdahl and other contributors +// Copyright (C) 2021-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -59,7 +59,7 @@ public: tl::expected remove(const Hash::Digest& key) override; private: - enum class Layout { flat, subdirs }; + enum class Layout : uint8_t { flat, subdirs }; std::string m_dir; std::optional m_umask; diff --git a/src/ccache/storage/remote/httpstorage.cpp b/src/ccache/storage/remote/httpstorage.cpp index 605fef15..fe0aa4b8 100644 --- a/src/ccache/storage/remote/httpstorage.cpp +++ b/src/ccache/storage/remote/httpstorage.cpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Joel Rosdahl and other contributors +// Copyright (C) 2021-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -54,7 +54,7 @@ public: tl::expected remove(const Hash::Digest& key) override; private: - enum class Layout { bazel, flat, subdirs }; + enum class Layout : uint8_t { bazel, flat, subdirs }; std::string m_url_path; httplib::Client m_http_client; diff --git a/src/ccache/util/assertions.hpp b/src/ccache/util/assertions.hpp index 5b632ad7..39304357 100644 --- a/src/ccache/util/assertions.hpp +++ b/src/ccache/util/assertions.hpp @@ -1,4 +1,4 @@ -// Copyright (C) 2020-2023 Joel Rosdahl and other contributors +// Copyright (C) 2020-2025 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -31,7 +31,8 @@ // release builds. #define ASSERT(condition) \ do { \ - if (!(condition)) { \ + if (condition) { \ + } else { \ util::handle_failed_assertion( \ __FILE__, __LINE__, CCACHE_FUNCTION, #condition); \ } \ diff --git a/src/ccache/util/logging.cpp b/src/ccache/util/logging.cpp index 9cbaf781..b9118661 100644 --- a/src/ccache/util/logging.cpp +++ b/src/ccache/util/logging.cpp @@ -70,7 +70,7 @@ print_fatal_error_and_exit() "ccache: error: Failed to write to {}: {}\n", logfile_path, strerror(errno)); - } catch (std::runtime_error&) { + } catch (std::runtime_error&) { // NOLINT: this is deliberate // Ignore since we can't do anything about it. } exit(EXIT_FAILURE); @@ -83,12 +83,12 @@ do_log(std::string_view message, bool bulk) if (!bulk || prefix[0] == '\0') { const auto now = util::TimePoint::now(); - snprintf(prefix, - sizeof(prefix), - "[%s.%06u %-5d] ", - util::format_iso8601_timestamp(now).c_str(), - static_cast(now.nsec_decimal_part() / 1000), - static_cast(getpid())); + (void)snprintf(prefix, + sizeof(prefix), + "[%s.%06u %-5d] ", + util::format_iso8601_timestamp(now).c_str(), + static_cast(now.nsec_decimal_part() / 1000), + static_cast(getpid())); } if (logfile) { diff --git a/src/ccache/util/string.cpp b/src/ccache/util/string.cpp index 4f63edb4..35453a69 100644 --- a/src/ccache/util/string.cpp +++ b/src/ccache/util/string.cpp @@ -193,12 +193,12 @@ format_iso8601_timestamp(const TimePoint& time, TimeZone time_zone) const auto tm = (time_zone == TimeZone::local ? util::localtime : util::gmtime)(time); if (tm) { - strftime(timestamp, sizeof(timestamp), "%Y-%m-%dT%H:%M:%S", &*tm); + (void)strftime(timestamp, sizeof(timestamp), "%Y-%m-%dT%H:%M:%S", &*tm); } else { - snprintf(timestamp, - sizeof(timestamp), - "%llu", - static_cast(time.sec())); + (void)snprintf(timestamp, + sizeof(timestamp), + "%llu", + static_cast(time.sec())); } return timestamp; } @@ -440,9 +440,9 @@ replace_first(const std::string_view string, std::string result; const auto pos = string.find(from); if (pos != std::string_view::npos) { - result.append(string.data(), pos); - result.append(to.data(), to.length()); - result.append(string.data() + pos + from.size()); + result.append(string.substr(0, pos)); + result.append(to); + result.append(string.substr(pos + from.size())); } else { result = std::string(string); }