From a7eeeb804797f859e75e7863a7c9ee01b9338929 Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Sat, 4 Nov 2023 11:51:16 -0600 Subject: [PATCH] Adds a clang-tidy, and its cleanups --- code/include/swoc/MemArena.h | 4 ++-- code/include/swoc/bwf_base.h | 2 +- code/src/ArenaWriter.cc | 3 ++- code/src/Errata.cc | 5 ++-- code/src/MemArena.cc | 24 ++++++++++++-------- code/src/RBTree.cc | 9 +++++--- code/src/TextView.cc | 5 ++-- code/src/bw_format.cc | 21 +++++++++-------- code/src/bw_ip_format.cc | 12 ++++++---- code/src/swoc_file.cc | 13 ++++++----- code/src/swoc_ip.cc | 7 +++--- tools/clang-tidy.conf | 44 ++++++++++++++++++++++++++++++++++++ 12 files changed, 107 insertions(+), 42 deletions(-) create mode 100644 tools/clang-tidy.conf diff --git a/code/include/swoc/MemArena.h b/code/include/swoc/MemArena.h index eabd0f17..6c1bc0fd 100644 --- a/code/include/swoc/MemArena.h +++ b/code/include/swoc/MemArena.h @@ -201,7 +201,7 @@ class MemArena MemArena(self_type const &that) = delete; /// Allow moving the arena. - MemArena(self_type &&that); + MemArena(self_type &&that) noexcept; /// Destructor. ~MemArena(); @@ -210,7 +210,7 @@ class MemArena self_type & operator=(self_type const & that) = delete; /// Move assignment. - self_type & operator=(self_type &&that); + self_type & operator=(self_type &&that) noexcept; /** Make a self-contained instance. * diff --git a/code/include/swoc/bwf_base.h b/code/include/swoc/bwf_base.h index 75e7a7de..58f81527 100644 --- a/code/include/swoc/bwf_base.h +++ b/code/include/swoc/bwf_base.h @@ -111,7 +111,7 @@ struct Spec { static const struct Property { Property(); ///< Default constructor, creates initialized flag set. /// Flag storage, indexed by character value. - uint8_t _data[0x100]; + uint8_t _data[0x100]{}; /// Flag mask values. static constexpr uint8_t ALIGN_MASK = 0x0F; ///< Alignment type. static constexpr uint8_t TYPE_CHAR = 0x10; ///< A valid type character. diff --git a/code/src/ArenaWriter.cc b/code/src/ArenaWriter.cc index c25265ed..e165df2c 100644 --- a/code/src/ArenaWriter.cc +++ b/code/src/ArenaWriter.cc @@ -44,4 +44,5 @@ ArenaWriter::realloc(size_t n) { memcpy(_buffer, text.data(), text.size()); } -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/code/src/Errata.cc b/code/src/Errata.cc index 9ef11fe9..036c9b41 100644 --- a/code/src/Errata.cc +++ b/code/src/Errata.cc @@ -94,7 +94,7 @@ Errata::note_s(std::optional severity, std::string_view text) { Errata & Errata::note_localized(std::string_view const &text, std::optional severity) { auto d = this->data(); - Annotation *n = d->_arena.make(text, severity); + auto *n = d->_arena.make(text, severity); d->_notes.append(n); return *this; } @@ -185,4 +185,5 @@ operator<<(std::ostream &os, Errata const &err) { return err.write(os); } -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/code/src/MemArena.cc b/code/src/MemArena.cc index 033f209b..451169fa 100644 --- a/code/src/MemArena.cc +++ b/code/src/MemArena.cc @@ -35,7 +35,7 @@ MemArena::MemArena(MemSpan static_block) { // integral values in @a that. MemArena::MemArena(swoc::MemArena::self_type &&that) - : _active_allocated(that._active_allocated), + noexcept : _active_allocated(that._active_allocated), _active_reserved(that._active_reserved), _frozen_allocated(that._frozen_allocated), _frozen_reserved(that._frozen_reserved), @@ -57,7 +57,7 @@ MemArena::construct_self_contained(size_t n) { } MemArena & -MemArena::operator=(swoc::MemArena::self_type &&that) { +MemArena::operator=(swoc::MemArena::self_type &&that) noexcept { this->clear(); std::swap(_active_allocated, that._active_allocated); std::swap(_active_reserved, that._active_reserved); @@ -155,10 +155,11 @@ MemArena::require(size_t n, size_t align) { // Search back through the list until a full block is hit, which is a miss. while (spot != _active.end() && !spot->satisfies(n, align)) { - if (spot->is_full()) + if (spot->is_full()) { spot = _active.end(); - else + } else { ++spot; +} } if (spot == _active.end()) { // no block has enough free space block = this->make_block(n); // assuming a new block is sufficiently aligned. @@ -178,8 +179,9 @@ MemArena::destroy_active() { auto sb = _static_block; // C++20 nonsense - capture of @a this is incompatible with C++17. _active .apply([=](Block *b) { - if (b != sb) + if (b != sb) { delete b; +} }) .clear(); } @@ -189,8 +191,9 @@ MemArena::destroy_frozen() { auto sb = _static_block; // C++20 nonsense - capture of @a this is incompatible with C++17. _frozen .apply([=](Block *b) { - if (b != sb) + if (b != sb) { delete b; +} }) .clear(); } @@ -228,14 +231,16 @@ MemArena::~MemArena() { while (bf) { Block *b = bf; bf = bf->_link._next; - if (b != sb) + if (b != sb) { delete b; +} } while (ba) { Block *b = ba; ba = ba->_link._next; - if (b != sb) + if (b != sb) { delete b; +} } } @@ -254,4 +259,5 @@ MemArena::do_is_equal(std::pmr::memory_resource const &that) const noexcept { } #endif -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/code/src/RBTree.cc b/code/src/RBTree.cc index 28416a99..7e708d54 100644 --- a/code/src/RBTree.cc +++ b/code/src/RBTree.cc @@ -229,7 +229,7 @@ RBNode * RBNode::rebalance_after_remove(Color c, //!< The color of the removed node Direction d //!< Direction of removed node from its parent ) { - self_type *root; + self_type *root = nullptr; if (Color::BLACK == c) { // only rebalance if too much black self_type *n = this; @@ -341,10 +341,13 @@ RBNode::validate() { auto RBNode::left_most_descendant() const -> self_type * { const self_type *n = this; - while (n->_left) + while (n->_left) { n = n->_left; +} return const_cast(n); } -}}} // namespace swoc::SWOC_VERSION_NS::detail +} // namespace detail +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/code/src/TextView.cc b/code/src/TextView.cc index 4262b633..f8da1112 100644 --- a/code/src/TextView.cc +++ b/code/src/TextView.cc @@ -80,7 +80,7 @@ svtou(TextView src, TextView *out, int base) { } if (src.ltrim_if(&isspace).size()) { auto origin = src.data(); - int8_t v; + int8_t v = 0; // If base is 0, it wasn't specified - check for standard base prefixes if (0 == base) { base = 10; @@ -220,7 +220,8 @@ svtod(swoc::TextView text, swoc::TextView *parsed) { // Do the template instantiations. template std::ostream &TextView::stream_write(std::ostream &, const TextView &) const; -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc namespace std { ostream & diff --git a/code/src/bw_format.cc b/code/src/bw_format.cc index c586b887..df53f008 100644 --- a/code/src/bw_format.cc +++ b/code/src/bw_format.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -190,7 +191,7 @@ Spec::parse(TextView fmt) { /// @return @c true if a specifier was parsed, @c false if not. bool Format::TextViewExtractor::parse(TextView &fmt, std::string_view &literal, std::string_view &specifier) { - TextView::size_type off; + TextView::size_type off = 0; // Check for brace delimiters. off = fmt.find_if([](char c) { return '{' == c || '}' == c; }); @@ -524,13 +525,13 @@ Format_Float(BufferWriter &w, Spec const &spec, double f, bool negative_p) { return w; } - uint64_t whole_part = static_cast(f); + auto whole_part = static_cast(f); if (whole_part == f || spec._prec == 0) { // integral return Format_Integer(w, spec, whole_part, negative_p); } static constexpr char dec = '.'; - double frac; + double frac = NAN; size_t l = 0; size_t r = 0; char whole[std::numeric_limits::digits10 + 1]; @@ -549,7 +550,7 @@ Format_Float(BufferWriter &w, Spec const &spec, double f, bool negative_p) { // Shift the floating point based on the precision. Used to convert // trailing fraction into an integer value. - uint64_t shift; + uint64_t shift = 0; if (precision < POWERS_OF_TEN.size()) { shift = POWERS_OF_TEN[precision]; } else { // not precomputed. @@ -559,7 +560,7 @@ Format_Float(BufferWriter &w, Spec const &spec, double f, bool negative_p) { } } - uint64_t frac_part = static_cast(frac * shift + 0.5 /* rounding */); + auto frac_part = static_cast(frac * shift + 0.5 /* rounding */); l = bwf::To_Radix<10>(whole_part, whole, sizeof(whole), bwf::LOWER_DIGITS); r = bwf::To_Radix<10>(frac_part, fraction, sizeof(fraction), bwf::LOWER_DIGITS); @@ -637,7 +638,7 @@ Format::is_literal() const { return true; } -NameBinding::~NameBinding() {} +NameBinding::~NameBinding() = default; } // namespace bwf @@ -716,8 +717,9 @@ bwformat(BufferWriter &w, bwf::Spec const &spec, MemSpan const &span TextView view{span.rebind()}; bool space_p = false; while (view) { - if (space_p) + if (space_p) { w.write(' '); +} space_p = true; if (spec._radix_lead_p) { w.write('0').write(digits[33]); @@ -916,7 +918,7 @@ bwformat(BufferWriter &w, bwf::Spec const &spec, bwf::Date const &date) { if (spec.has_numeric_type()) { bwformat(w, spec, date._epoch); } else { - struct tm t; + struct tm t{}; auto r = w.remaining(); size_t n{0}; // Verify @a fmt is null terminated, even outside the bounds of the view. @@ -995,7 +997,8 @@ bwformat(BufferWriter &w, bwf::Spec const& spec, bwf::UnHex const& obj) { return w; } -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc namespace std { ostream & diff --git a/code/src/bw_ip_format.cc b/code/src/bw_ip_format.cc index 6072275f..53b54547 100644 --- a/code/src/bw_ip_format.cc +++ b/code/src/bw_ip_format.cc @@ -141,10 +141,12 @@ bwformat(BufferWriter &w, Spec const &spec, sockaddr const *addr) { w.print("*Invalid IP family [{}]*", addr->sa_family); break; } - if (bracket_p) + if (bracket_p) { w.write(']'); - if (port_p) +} + if (port_p) { w.write(':'); +} } if (port_p) { if (local_numeric_fill_p) { @@ -158,8 +160,9 @@ bwformat(BufferWriter &w, Spec const &spec, sockaddr const *addr) { } if (family_p) { local_spec._min = 0; - if (addr_p || port_p) + if (addr_p || port_p) { w.write(' '); +} if (spec.has_numeric_type()) { bwformat(w, local_spec, static_cast(addr->sa_family)); } else { @@ -382,4 +385,5 @@ bwformat(BufferWriter &w, Spec const &spec, IPMask const &mask) { return bwformat(w, spec, mask.width()); } -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/code/src/swoc_file.cc b/code/src/swoc_file.cc index 93086c27..4ab6da65 100644 --- a/code/src/swoc_file.cc +++ b/code/src/swoc_file.cc @@ -360,12 +360,12 @@ uintmax_t remove_all(const path &p, std::error_code &ec) { // coverity TOCTOU - issue is doing stat before doing operation. Stupid complaint, ignore. - DIR *dir; - struct dirent *entry; + DIR *dir = nullptr; + struct dirent *entry = nullptr; std::error_code err; uintmax_t zret = 0; - struct ::stat s; + struct ::stat s{}; if (p.empty()) { ec = std::error_code(EINVAL, std::system_category()); return zret; @@ -414,7 +414,7 @@ remove_all(const path &p, std::error_code &ec) bool remove(path const& p, std::error_code &ec) { // coverity TOCTOU - issue is doing stat before doing operation. Stupid complaint, ignore. - struct ::stat fs; + struct ::stat fs{}; if (p.empty()) { ec = std::error_code(EINVAL, std::system_category()); } else if (::stat(p.c_str(), &fs) < 0) { @@ -442,7 +442,7 @@ load(const path &p, std::error_code &ec) { if (unique_fd fd(::open(p.c_str(), O_RDONLY)) ; fd < 0) { ec = std::error_code(errno, std::system_category()); } else { - struct stat info; + struct stat info{}; if (0 != ::fstat(fd, &info)) { ec = std::error_code(errno, std::system_category()); } else { @@ -464,4 +464,5 @@ bwformat(BufferWriter &w, bwf::Spec const &spec, file::path const &p) { return bwformat(w, spec, p.string()); } -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/code/src/swoc_ip.cc b/code/src/swoc_ip.cc index 6d25a5b4..e437e24a 100644 --- a/code/src/swoc_ip.cc +++ b/code/src/swoc_ip.cc @@ -802,8 +802,8 @@ IPSrv::load(swoc::TextView text) { } -IPSrv::IPSrv(IPAddr addr, in_port_t port) { - _family = addr.family(); +IPSrv::IPSrv(IPAddr addr, in_port_t port) : _family(addr.family()) { + if (addr.is_ip4()) { _srv._ip4.assign(addr.ip4(), port); } else if (addr.is_ip6()) { @@ -1254,4 +1254,5 @@ IPRangeView::operator==(IPRangeView::self_type const &that) const { return false; } -}} // namespace swoc::SWOC_VERSION_NS +} // namespace SWOC_VERSION_NS +} // namespace swoc diff --git a/tools/clang-tidy.conf b/tools/clang-tidy.conf new file mode 100644 index 00000000..32b081d0 --- /dev/null +++ b/tools/clang-tidy.conf @@ -0,0 +1,44 @@ +--- +Checks: "cppcoreguidelines-*,performance-*,modernize-*,google-*,clang-diagnostic-*,clang-analyzer-*,-google-readability-casting,-google-explicit-constructor,-modernize-use-trailing-return-type,-google-global-names-in-headers,-modernize-avoid-c-arrays" +WarningsAsErrors: "" +HeaderFilterRegex: "(Cript|src)/.*" +AnalyzeTemporaryDtors: false +FormatStyle: none +CheckOptions: + - key: llvm-else-after-return.WarnOnConditionVariables + value: "true" + - key: modernize-loop-convert.MinConfidence + value: reasonable + - key: modernize-replace-auto-ptr.IncludeStyle + value: llvm + - key: cert-str34-c.DiagnoseSignedUnsignedCharComparisons + value: "true" + - key: google-readability-namespace-comments.ShortNamespaceLines + value: "10" + - key: cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField + value: "false" + - key: cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic + value: "true" + - key: cert-dcl16-c.NewSuffixes + value: "L;LL;LU;LLU" + - key: google-readability-braces-around-statements.ShortStatementLines + value: "1" + - key: modernize-pass-by-value.IncludeStyle + value: llvm + - key: google-readability-namespace-comments.SpacesBeforeComments + value: "2" + - key: modernize-loop-convert.MaxCopySize + value: "16" + - key: cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors + value: "true" + - key: modernize-use-nullptr.NullMacros + value: "NULL" + - key: llvm-qualified-auto.AddConstToQualified + value: "true" + - key: modernize-loop-convert.NamingStyle + value: CamelCase + - key: llvm-else-after-return.WarnOnUnfixable + value: "true" + - key: google-readability-function-size.StatementThreshold + value: "800" +---