From ac90ed1fca5af47ec040c7576db576b253382c22 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sun, 3 Mar 2019 12:43:54 +0000 Subject: Lots more clang tidy fixes --- libadhocutil/cache.h | 6 ++++++ libadhocutil/compileTimeFormatter.h | 3 +++ libadhocutil/exception.h | 2 +- libadhocutil/nvpParse.h | 8 ++++---- libadhocutil/nvpParse.ll | 3 +++ libadhocutil/resourcePool.h | 2 -- libadhocutil/resourcePool.impl.h | 19 ------------------- libadhocutil/unittests/Jamfile.jam | 6 ------ libadhocutil/unittests/testBuffer.cpp | 6 +++--- libadhocutil/unittests/testCache.cpp | 2 ++ libadhocutil/unittests/testCompileTimeFormatter.cpp | 14 ++++++++++---- libadhocutil/unittests/testException.cpp | 2 ++ libadhocutil/unittests/testFactory.cpp | 1 + libadhocutil/unittests/testFileUtils.cpp | 6 +++--- libadhocutil/unittests/testLazyPointer.cpp | 2 +- libadhocutil/unittests/testMapFinds.cpp | 2 +- libadhocutil/unittests/testNvpParse.cpp | 4 ++-- libadhocutil/unittests/testProcessPipes.cpp | 6 +++--- libadhocutil/unittests/testResourcePool.cpp | 6 ++++++ libadhocutil/uriParse.h | 2 +- 20 files changed, 52 insertions(+), 50 deletions(-) diff --git a/libadhocutil/cache.h b/libadhocutil/cache.h index 66b4a7c..933c8ff 100644 --- a/libadhocutil/cache.h +++ b/libadhocutil/cache.h @@ -19,8 +19,14 @@ class DLL_PUBLIC Cacheable { public: typedef const std::shared_ptr Value; Cacheable(const K & k, time_t validUntil); + Cacheable(const Cacheable &) = delete; + Cacheable(Cacheable &&) = default; + virtual ~Cacheable() = default; + void operator=(const Cacheable &) = delete; + void operator=(Cacheable &&) = delete; + const K key; const time_t validUntil; diff --git a/libadhocutil/compileTimeFormatter.h b/libadhocutil/compileTimeFormatter.h index 785f255..9d73868 100644 --- a/libadhocutil/compileTimeFormatter.h +++ b/libadhocutil/compileTimeFormatter.h @@ -180,9 +180,11 @@ namespace AdHoc { { if (pos != L) { constexpr auto ph = strchrnul(); + // NOLINTNEXTLINE(hicpp-braces-around-statements,bugprone-suspicious-semicolon) if constexpr (ph != pos) { appendStream(s, (S + pos), ph - pos); } + // NOLINTNEXTLINE(hicpp-braces-around-statements,bugprone-suspicious-semicolon) if constexpr (ph != L) { packAndWrite(s, pn...); } @@ -192,6 +194,7 @@ namespace AdHoc { template static inline void packAndWrite(stream & s, const Pn & ... pn) { + // NOLINTNEXTLINE(hicpp-braces-around-statements) if constexpr (ph + off == L || sizeof...(Pck) == 32) { StreamWriter::write(s, pn...); } diff --git a/libadhocutil/exception.h b/libadhocutil/exception.h index 92ed65f..2152446 100644 --- a/libadhocutil/exception.h +++ b/libadhocutil/exception.h @@ -13,7 +13,7 @@ namespace AdHoc { /// Wrapper constructor to pass to BaseException //@param t parameters to pass. template - Exception(const T & ... t) : BaseException(t...) { } + explicit Exception(const T & ... t) : BaseException(t...) { } /// Override of std::exception::what() to create text as required. inline const char * what() const noexcept override diff --git a/libadhocutil/nvpParse.h b/libadhocutil/nvpParse.h index 3c6b03a..df3982d 100644 --- a/libadhocutil/nvpParse.h +++ b/libadhocutil/nvpParse.h @@ -1,7 +1,6 @@ #ifndef ADHOCUTIL_REFLECTION_H #define ADHOCUTIL_REFLECTION_H -#include #include #include #include @@ -30,8 +29,8 @@ class NvpParse : public yyFlexLexer { ValueNotFound(const std::string &); }; - typedef std::function AssignFunc; - typedef std::map AssignMap; + using AssignFunc = std::function; + using AssignMap = std::map; template class TargetBase { @@ -60,6 +59,7 @@ class NvpParse : public yyFlexLexer { }; /// @endcond + // NOLINTNEXTLINE(bugprone-macro-parentheses) #define NvpTarget(T) std::map>> #define NvpValue(c, m) { #m, std::make_shared<::AdHoc::NvpParse::Target>(&c::m) } @@ -85,7 +85,7 @@ class NvpParse : public yyFlexLexer { private: NvpParse(std::istream & in, const AssignMap &); - ~NvpParse(); + ~NvpParse() override; int yylex() override; void LexerError(const char * msg) override; diff --git a/libadhocutil/nvpParse.ll b/libadhocutil/nvpParse.ll index 93b45e2..d7b88b2 100644 --- a/libadhocutil/nvpParse.ll +++ b/libadhocutil/nvpParse.ll @@ -10,6 +10,9 @@ #include "nvpParse.h" #pragma GCC diagnostic ignored "-Wsign-compare" #pragma GCC diagnostic ignored "-Wimplicit-fallthrough" +#if __clang__ +#pragma GCC diagnostic ignored "-Wnull-conversion" +#endif %} element [a-zA-Z][a-zA-Z0-9_-]* diff --git a/libadhocutil/resourcePool.h b/libadhocutil/resourcePool.h index bcc870b..1321201 100644 --- a/libadhocutil/resourcePool.h +++ b/libadhocutil/resourcePool.h @@ -91,8 +91,6 @@ namespace AdHoc { protected: /// Create a new resource instance to add to the pool. virtual std::shared_ptr createResource() const = 0; - /// Destroy an existing resource (defaults to delete). - virtual void destroyResource(Resource const *) const throw(); /// Test a cached resource is still suitable for use before re-use (defaults to no-op). virtual void testResource(Resource const *) const; /// Test a cached resource is still suitable for use on return (defaults to no-op). diff --git a/libadhocutil/resourcePool.impl.h b/libadhocutil/resourcePool.impl.h index 1f605a5..a565ae1 100644 --- a/libadhocutil/resourcePool.impl.h +++ b/libadhocutil/resourcePool.impl.h @@ -131,21 +131,11 @@ namespace AdHoc { template ResourcePool::~ResourcePool() { - for (auto & r : available) { - destroyResource(r.get()); - } for (auto & r : inUse) { - destroyResource(std::get<0>(*r.second).get()); std::get<1>(*r.second) = nullptr; } } - template - void - ResourcePool::destroyResource(R const *) const throw() - { - } - template void ResourcePool::testResource(R const *) const @@ -195,9 +185,6 @@ namespace AdHoc { ResourcePool::idle() { Lock(lock); - for (auto & r : available) { - destroyResource(r.get()); - } available.clear(); } @@ -246,7 +233,6 @@ namespace AdHoc { return ro; } catch (...) { - destroyResource(r.get()); available.pop_front(); } } @@ -266,12 +252,8 @@ namespace AdHoc { if (available.size() < keep) { available.push_back(r); } - else { - destroyResource(r.get()); - } } catch (...) { - destroyResource(r.get()); } poolSize.notify(); } @@ -282,7 +264,6 @@ namespace AdHoc { { Lock(lock); removeFrom(r, inUse); - destroyResource(r.get()); poolSize.notify(); } diff --git a/libadhocutil/unittests/Jamfile.jam b/libadhocutil/unittests/Jamfile.jam index 8660a87..02a9275 100644 --- a/libadhocutil/unittests/Jamfile.jam +++ b/libadhocutil/unittests/Jamfile.jam @@ -7,12 +7,6 @@ type.register TEXT : txt ; generators.register-standard xxd.h : TEXT : H ; -project - : requirements - tidy:hicpp-* - tidy:performance-* - ; - actions xxd.h { ( cd ./$(2:D) && xxd -i $(2:B).txt ) > $(1[1]) diff --git a/libadhocutil/unittests/testBuffer.cpp b/libadhocutil/unittests/testBuffer.cpp index d421443..d3a3dcf 100644 --- a/libadhocutil/unittests/testBuffer.cpp +++ b/libadhocutil/unittests/testBuffer.cpp @@ -78,9 +78,9 @@ BOOST_AUTO_TEST_CASE( writeto ) .append(std::string(" b")) .appendf(" num %d", 1) .appendbf(" num %d", 2); - char buf[23]; - b.writeto(buf, 23, 0); - BOOST_REQUIRE_EQUAL(0, memcmp(buf, "string a b num 1 num 2", 23)); + std::string buf(22, '\0'); + b.writeto(buf.data(), 23, 0); + BOOST_REQUIRE_EQUAL(buf, "string a b num 1 num 2"); } BOOST_AUTO_TEST_CASE( operators ) diff --git a/libadhocutil/unittests/testCache.cpp b/libadhocutil/unittests/testCache.cpp index dc7bd6b..b19f587 100644 --- a/libadhocutil/unittests/testCache.cpp +++ b/libadhocutil/unittests/testCache.cpp @@ -5,8 +5,10 @@ #include "cache.h" #include "cache.impl.h" +// NOLINTNEXTLINE(hicpp-special-member-functions) class Obj { public: + // NOLINTNEXTLINE(hicpp-explicit-conversions) Obj(int i) : v(i) { } void operator=(const Obj &) = delete; bool operator==(const int & i) const { diff --git a/libadhocutil/unittests/testCompileTimeFormatter.cpp b/libadhocutil/unittests/testCompileTimeFormatter.cpp index 12b72b8..39a996b 100644 --- a/libadhocutil/unittests/testCompileTimeFormatter.cpp +++ b/libadhocutil/unittests/testCompileTimeFormatter.cpp @@ -35,6 +35,7 @@ namespace AdHoc { template static void write(stream & s, const P & p, const Pn & ... pn) { + // NOLINTNEXTLINE(hicpp-no-array-decay) s << "-( " << p << " )-"; StreamWriter::next(s, pn...); } @@ -45,6 +46,7 @@ namespace AdHoc { template static void write(stream & s, const P & p, const Pn & ... pn) { + // NOLINTNEXTLINE(hicpp-no-array-decay) s << "---( " << p << " )---"; StreamWriter::next(s, pn...); } @@ -57,6 +59,7 @@ namespace AdHoc { { // NOLINTNEXTLINE(bugprone-string-constructor) std::string d(dashes, '-'); + // NOLINTNEXTLINE(hicpp-no-array-decay) s << d << "( " << p << " )" << d; StreamWriter::next(s, pn...); } @@ -68,6 +71,7 @@ namespace AdHoc { static void write(stream & s, int width, const P & p, const Pn & ... pn) { std::stringstream buf; + // NOLINTNEXTLINE(hicpp-no-array-decay) buf << p; std::string spaces(width - buf.str().length(), ' '); s << spaces << buf.str(); @@ -277,7 +281,7 @@ BOOST_AUTO_TEST_CASE( lorem_ipsum ) auto s = LIF::get(); BOOST_CHECK_EQUAL(s.length(), lorem_ipsum_txt_len); AdHoc::FileUtils::MemMap li(rootDir / "lorem-ipsum.txt"); - auto sv = li.sv::type>(); + auto sv = li.sv::type>(); BOOST_CHECK_EQUAL_COLLECTIONS(s.begin(), s.end(), sv.begin(), sv.end()); } @@ -285,6 +289,7 @@ namespace AdHoc { template<> inline void appendStream(FILE & strm, const char * const p, size_t n) { + // NOLINTNEXTLINE(hicpp-no-array-decay) BOOST_VERIFY(fwrite(p, n, 1, &strm) == 1); } } @@ -292,7 +297,7 @@ namespace AdHoc { FILE & operator<<(FILE & strm, const char * const p) { - BOOST_VERIFY(fputs(p, &strm) != EOF); + BOOST_CHECK_NE(fputs(p, &strm), EOF); return strm; } @@ -307,6 +312,7 @@ BOOST_AUTO_TEST_CASE( filestar ) fclose(strm); BOOST_CHECK_EQUAL(len, 22); BOOST_CHECK_EQUAL(buf, "First file, then star."); + // NOLINTNEXTLINE(hicpp-no-malloc) free(buf); } @@ -338,10 +344,10 @@ static_assert(419 == decdigits<'0', '4', '1', '9'>()); auto str = NAME ## fmtr::get(__VA_ARGS__); \ char * buf = NULL; \ int len = asprintf(&buf, FMT, __VA_ARGS__); \ - BOOST_REQUIRE(buf); \ + auto bufp = std::unique_ptr(buf, std::free); \ + BOOST_REQUIRE(bufp); \ BOOST_CHECK_EQUAL(str.length(), len); \ BOOST_CHECK_EQUAL(str, buf); \ - free(buf); \ } \ } diff --git a/libadhocutil/unittests/testException.cpp b/libadhocutil/unittests/testException.cpp index 63fc2a3..2533301 100644 --- a/libadhocutil/unittests/testException.cpp +++ b/libadhocutil/unittests/testException.cpp @@ -49,11 +49,13 @@ class Ex3 : public AdHoc::StdException { void failing1() { + // NOLINTNEXTLINE(hicpp-no-array-decay) throw Ex1(__PRETTY_FUNCTION__, 1); } void failing2() { + // NOLINTNEXTLINE(hicpp-no-array-decay) throw Ex2(__PRETTY_FUNCTION__, 2); } diff --git a/libadhocutil/unittests/testFactory.cpp b/libadhocutil/unittests/testFactory.cpp index b8b910d..868e5b8 100644 --- a/libadhocutil/unittests/testFactory.cpp +++ b/libadhocutil/unittests/testFactory.cpp @@ -8,6 +8,7 @@ using namespace AdHoc; +// NOLINTNEXTLINE(hicpp-special-member-functions) class BaseThing { public: BaseThing(int, std::string * n) : diff --git a/libadhocutil/unittests/testFileUtils.cpp b/libadhocutil/unittests/testFileUtils.cpp index 2a823df..7cd220c 100644 --- a/libadhocutil/unittests/testFileUtils.cpp +++ b/libadhocutil/unittests/testFileUtils.cpp @@ -46,13 +46,13 @@ T moveTest(P ... p) class Base { public: - Base(AdHoc::FileUtils::FileHandle h) : fh(std::move(h)) { } + explicit Base(AdHoc::FileUtils::FileHandle h) : fh(std::move(h)) { } AdHoc::FileUtils::FileHandle fh; }; class Sub : public Base { public: - Sub(AdHoc::FileUtils::FileHandle h) : Base(std::move(h)) { } + explicit Sub(AdHoc::FileUtils::FileHandle h) : Base(std::move(h)) { } }; BOOST_AUTO_TEST_CASE( movePassThrough ) @@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE( movePassThrough ) auto y = std::make_unique(std::move(f)); BOOST_REQUIRE_EQUAL(y->fh, ffd); REQUIRE_VALID_FH(y->fh); - // NOLINTNEXTLINE(bugprone-use-after-move) + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) BOOST_REQUIRE_EQUAL(f.fh, -1); REQUIRE_INVALID_FH(f.fh); y.reset(); diff --git a/libadhocutil/unittests/testLazyPointer.cpp b/libadhocutil/unittests/testLazyPointer.cpp index a659548..52e5c79 100644 --- a/libadhocutil/unittests/testLazyPointer.cpp +++ b/libadhocutil/unittests/testLazyPointer.cpp @@ -7,7 +7,7 @@ using namespace AdHoc; class Test { public: - Test(int v) : + explicit Test(int v) : val(v) { } diff --git a/libadhocutil/unittests/testMapFinds.cpp b/libadhocutil/unittests/testMapFinds.cpp index a787676..d3d0e34 100644 --- a/libadhocutil/unittests/testMapFinds.cpp +++ b/libadhocutil/unittests/testMapFinds.cpp @@ -11,7 +11,7 @@ using namespace AdHoc; class NotFound : std::runtime_error { public: - NotFound(int key) : + explicit NotFound(int key) : std::runtime_error(boost::lexical_cast("Key not found: %d", key)) { } diff --git a/libadhocutil/unittests/testNvpParse.cpp b/libadhocutil/unittests/testNvpParse.cpp index 9e3bf06..619123a 100644 --- a/libadhocutil/unittests/testNvpParse.cpp +++ b/libadhocutil/unittests/testNvpParse.cpp @@ -9,8 +9,8 @@ class TestTarget { public: std::string a; std::string b; - int c; - double d; + int c {0}; + double d {0}; }; NvpTarget(TestTarget) TestTargetMap { diff --git a/libadhocutil/unittests/testProcessPipes.cpp b/libadhocutil/unittests/testProcessPipes.cpp index f618940..b568984 100644 --- a/libadhocutil/unittests/testProcessPipes.cpp +++ b/libadhocutil/unittests/testProcessPipes.cpp @@ -13,11 +13,11 @@ BOOST_AUTO_TEST_CASE ( readfind ) BOOST_REQUIRE_EQUAL(pp.fdIn(), -1); BOOST_REQUIRE_NE(pp.fdOut(), -1); BOOST_REQUIRE_NE(pp.fdError(), -1); - char buf[BUFSIZ]; - ssize_t bytes = read(pp.fdOut(), buf, BUFSIZ); + std::string buf(BUFSIZ, '\0'); + ssize_t bytes = read(pp.fdOut(), buf.data(), BUFSIZ); BOOST_REQUIRE_MESSAGE(bytes > 0, "bytes = " << bytes); buf[bytes] = '\0'; - char * lnf = strstr(buf, "testProcessPipes.cpp"); + const char * lnf = strstr(buf.data(), "testProcessPipes.cpp"); BOOST_REQUIRE_MESSAGE(lnf, buf); int status; waitpid(pp.pid(), &status, 0); diff --git a/libadhocutil/unittests/testResourcePool.cpp b/libadhocutil/unittests/testResourcePool.cpp index 04e66b8..ae3e322 100644 --- a/libadhocutil/unittests/testResourcePool.cpp +++ b/libadhocutil/unittests/testResourcePool.cpp @@ -9,7 +9,9 @@ class MockResource { ~MockResource() { count -= 1; } MockResource(const MockResource &) = delete; + MockResource(MockResource &&) = delete; void operator=(const MockResource &) = delete; + void operator=(MockResource &&) = delete; bool valid() const { return true; } @@ -116,6 +118,7 @@ BOOST_AUTO_TEST_CASE( destroyPoolWhenInUse ) auto rp = new TRP(); auto rh1 = rp->get(); auto rh2 = rp->get(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) auto rh3 = rh1; delete rp; BOOST_REQUIRE(rh1->valid()); @@ -156,12 +159,14 @@ BOOST_AUTO_TEST_CASE( move ) { auto r2(std::move(r1)); BOOST_CHECK_EQUAL(pool.inUseCount(), 1); + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) BOOST_CHECK(!r1); BOOST_CHECK(r2); r1 = std::move(r2); BOOST_CHECK_EQUAL(pool.inUseCount(), 1); BOOST_CHECK(r1); + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) BOOST_CHECK(!r2); r2 = pool.get(); @@ -171,6 +176,7 @@ BOOST_AUTO_TEST_CASE( move ) r1 = std::move(r2); BOOST_CHECK_EQUAL(pool.inUseCount(), 1); BOOST_CHECK(r1); + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) BOOST_CHECK(!r2); } BOOST_CHECK_EQUAL(pool.inUseCount(), 1); diff --git a/libadhocutil/uriParse.h b/libadhocutil/uriParse.h index 309737f..e6fd0cb 100644 --- a/libadhocutil/uriParse.h +++ b/libadhocutil/uriParse.h @@ -16,7 +16,7 @@ namespace AdHoc { public: /// Constructor accepting a URI to parse. /// @param uri the URI to parse. - Uri(const std::string & uri); + explicit Uri(const std::string & uri); /// The scheme. std::string scheme; -- cgit v1.2.3