From 232501140c2d6fba164aa6757a816c7da569830a Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 27 Mar 2019 22:22:08 +0000 Subject: Add clang-tidy rules and fix all the things Note: something in here appears to break linking in release build --- Jamroot.jam | 7 ++ icespider/common/http.ice | 2 +- icespider/common/pathparts.cpp | 4 +- icespider/compile/routeCompiler.cpp | 132 +++++++++++++++++-------------- icespider/compile/routeCompiler.h | 24 +++--- icespider/core/Jamfile.jam | 2 +- icespider/core/core.cpp | 30 ++++--- icespider/core/exceptions.h | 4 +- icespider/core/hextable.c | 13 --- icespider/core/ihttpRequest.cpp | 34 +++++--- icespider/core/irouteHandler.cpp | 6 +- icespider/core/irouteHandler.h | 4 +- icespider/core/xwwwFormUrlEncoded.cpp | 58 +++++++++++--- icespider/core/xwwwFormUrlEncoded.h | 6 +- icespider/fileSessions/fileSessions.cpp | 47 +++++++---- icespider/testing/testRequest.cpp | 15 ++-- icespider/unittests/Jamfile.jam | 6 +- icespider/unittests/testApp.cpp | 16 ++-- icespider/unittests/testCompile.cpp | 3 +- icespider/unittests/testFcgi.cpp | 19 +++-- icespider/unittests/testFileSessions.cpp | 5 +- icespider/xslt/xsltStreamSerializer.cpp | 2 +- 22 files changed, 261 insertions(+), 178 deletions(-) delete mode 100644 icespider/core/hextable.c diff --git a/Jamroot.jam b/Jamroot.jam index 6fc8d34..8b18b94 100644 --- a/Jamroot.jam +++ b/Jamroot.jam @@ -15,6 +15,13 @@ project debug:"-W -Wall -Wextra -Werror -Wwrite-strings" coverage:"--coverage" coverage:"--coverage" + tidy:boost-* + tidy:bugprone-* + tidy:clang-* + tidy:misc-* + tidy:modernize-* + tidy:hicpp-* + tidy:performance-* ; build-project icespider ; diff --git a/icespider/common/http.ice b/icespider/common/http.ice index d535ee2..35c518d 100644 --- a/icespider/common/http.ice +++ b/icespider/common/http.ice @@ -6,7 +6,7 @@ module IceSpider { ["slicer:ignore"] local exception HttpException { - int code; + short code; string message; }; diff --git a/icespider/common/pathparts.cpp b/icespider/common/pathparts.cpp index 23e6f31..16c8d63 100644 --- a/icespider/common/pathparts.cpp +++ b/icespider/common/pathparts.cpp @@ -9,7 +9,9 @@ namespace IceSpider { path(p) { auto relp = p.substr(1); - if (relp.empty()) return; + if (relp.empty()) { + return; + } for (auto pi = ba::make_split_iterator(relp, ba::first_finder("/", ba::is_equal())); pi != decltype(pi)(); ++pi) { std::string_view pp(pi->begin(), pi->end() - pi->begin()); if (pp.front() == '{' && pp.back() == '}') { diff --git a/icespider/compile/routeCompiler.cpp b/icespider/compile/routeCompiler.cpp index 75737ee..b94606c 100644 --- a/icespider/compile/routeCompiler.cpp +++ b/icespider/compile/routeCompiler.cpp @@ -47,9 +47,11 @@ namespace IceSpider { } for (const auto & m : c->modules()) { auto op = findOperation(on, m, ns + m->name()); - if (op) return op; + if (op) { + return op; + } } - return NULL; + return nullptr; } Slice::OperationPtr @@ -57,7 +59,9 @@ namespace IceSpider { { for (const auto & u : us) { auto op = findOperation(on, u.second); - if (op) return op; + if (op) { + return op; + } } throw std::runtime_error("Find operation " + on + " failed."); } @@ -67,22 +71,30 @@ namespace IceSpider { { for (const auto & strct : c->structs()) { auto fqon = boost::algorithm::join(ns + strct->name(), "."); - if (fqon == tn) return { strct, NULL }; + if (fqon == tn) { + return { strct, nullptr }; + } auto t = findType(tn, strct, ns + strct->name()); } for (const auto & cls : c->classes()) { auto fqon = boost::algorithm::join(ns + cls->name(), "."); - if (fqon == tn) return { NULL, cls->declaration() }; + if (fqon == tn) { + return { nullptr, cls->declaration() }; + } auto t = findType(tn, cls, ns + cls->name()); // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDelete) - if (t.first || t.second) return t; + if (t.first || t.second) { + return t; + } } for (const auto & m : c->modules()) { auto t = findType(tn, m, ns + m->name()); // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDelete) - if (t.first || t.second) return t; + if (t.first || t.second) { + return t; + } } - return { NULL, NULL }; + return { nullptr, nullptr }; } RouteCompiler::Type @@ -91,13 +103,15 @@ namespace IceSpider { for (const auto & u : us) { // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDelete) auto t = findType(tn, u.second); - if (t.first || t.second) return t; + if (t.first || t.second) { + return t; + } } throw std::runtime_error("Find type " + tn + " failed."); } RouteCompiler::ParameterMap - RouteCompiler::findParameters(RoutePtr r, const Units & us) + RouteCompiler::findParameters(const RoutePtr & r, const Units & us) { RouteCompiler::ParameterMap pm; for (const auto & o : r->operations) { @@ -119,7 +133,7 @@ namespace IceSpider { } void - RouteCompiler::applyDefaults(RouteConfigurationPtr c, const Units & u) const + RouteCompiler::applyDefaults(const RouteConfigurationPtr & c, const Units & u) const { for (const auto & r : c->routes) { if (r.second->operation) { @@ -174,7 +188,7 @@ namespace IceSpider { fclose(out); fclose(outh); }, - NULL, + nullptr, [&output,&outputh]() { std::filesystem::remove(output); std::filesystem::remove(outputh); @@ -183,7 +197,7 @@ namespace IceSpider { } RouteCompiler::Units - RouteCompiler::loadUnits(RouteConfigurationPtr c) const + RouteCompiler::loadUnits(const RouteConfigurationPtr & c) const { RouteCompiler::Units units; AdHoc::ScopeExit uDestroy; @@ -200,18 +214,18 @@ namespace IceSpider { } std::vector cppArgs; for (const auto & p : searchPath) { - cppArgs.push_back("-I"); - cppArgs.push_back(p.string()); + cppArgs.emplace_back("-I"); + cppArgs.emplace_back(p.string()); } Slice::PreprocessorPtr icecpp = Slice::Preprocessor::create("IceSpider", realSlice.string(), cppArgs); FILE * cppHandle = icecpp->preprocess(false); - if (cppHandle == NULL) { + if (!cppHandle) { throw std::runtime_error("Preprocess failed"); } Slice::UnitPtr u = Slice::Unit::createUnit(false, false, false, false); - uDestroy.onFailure.push_back([u]() { u->destroy(); }); + uDestroy.onFailure.emplace_back([u]() { u->destroy(); }); int parseStatus = u->parse(realSlice.string(), cppHandle, false); @@ -233,8 +247,9 @@ namespace IceSpider { fprintbf(unsigned int indent, FILE * output, const X & ... x) { for (; indent > 0; --indent) { - fprintf(output, "\t"); + fputc('\t', output); } + // NOLINTNEXTLINE(hicpp-no-array-decay) fprintbf(output, x...); } @@ -262,7 +277,7 @@ namespace IceSpider { } void - RouteCompiler::registerOutputSerializers(FILE * output, RoutePtr r) const + RouteCompiler::registerOutputSerializers(FILE * output, const RoutePtr & r) const { for (const auto & os : r->outputSerializers) { fprintbf(4, output, "addRouteSerializer(%s,\n", @@ -271,32 +286,32 @@ namespace IceSpider { outputSerializerClass(os)); for (auto p = os.second->params.begin(); p != os.second->params.end(); ++p) { if (p != os.second->params.begin()) { - fprintf(output, ", "); + fputs(", ", output); } fputs(p->c_str(), output); } - fprintf(output, "));\n"); + fputs("));\n", output); } } void - RouteCompiler::processConfiguration(FILE * output, FILE * outputh, const std::string & name, RouteConfigurationPtr c, const Units & units) const + RouteCompiler::processConfiguration(FILE * output, FILE * outputh, const std::string & name, const RouteConfigurationPtr & c, const Units & units) const { - fprintf(output, "// This source files was generated by IceSpider.\n"); + fputs("// This source files was generated by IceSpider.\n", output); fprintbf(output, "// Configuration name: %s\n\n", c->name); - fprintf(outputh, "// This source files was generated by IceSpider.\n"); + fputs("// This source files was generated by IceSpider.\n", outputh); fprintbf(outputh, "// Configuration name: %s\n\n", c->name); - fprintf(output, "// Configuration headers.\n"); + fputs("// Configuration headers.\n", output); fprintbf(output, "#include \"%s.h\"\n", name); - fprintf(outputh, "// Standard headers.\n"); - fprintf(outputh, "#include \n"); - fprintf(outputh, "#include \n"); - fprintf(outputh, "#include \n"); + fputs("// Standard headers.\n", outputh); + fputs("#include \n", outputh); + fputs("#include \n", outputh); + fputs("#include \n", outputh); - fprintf(outputh, "\n// Interface headers.\n"); + fputs("\n// Interface headers.\n", outputh); for (const auto & s : c->slices) { std::filesystem::path slicePath(s); slicePath.replace_extension(".h"); @@ -304,7 +319,7 @@ namespace IceSpider { } if (!c->headers.empty()) { - fprintf(outputh, "\n// Extra headers.\n"); + fputs("\n// Extra headers.\n", outputh); for (const auto & h : c->headers) { fprintbf(outputh, "#include <%s>\n", h); } @@ -313,16 +328,16 @@ namespace IceSpider { processBases(output, outputh, c, units); processRoutes(output, c, units); - fprintf(output, "\n// End generated code.\n"); + fputs("\n// End generated code.\n", output); } void - RouteCompiler::processBases(FILE * output, FILE * outputh, RouteConfigurationPtr c, const Units & u) const + RouteCompiler::processBases(FILE * output, FILE * outputh, const RouteConfigurationPtr & c, const Units & u) const { - fprintf(outputh, "\n"); + fputs("\n", outputh); fprintbf(outputh, "namespace %s {\n", c->name); fprintbf(1, outputh, "// Base classes.\n\n"); - fprintf(output, "\n"); + fputs("\n", output); fprintbf(output, "namespace %s {\n", c->name); fprintbf(1, output, "// Base classes.\n\n"); for (const auto & r : c->routeBases) { @@ -337,15 +352,15 @@ namespace IceSpider { { fprintbf(1, outputh, "class %s {\n", b.first); fprintbf(2, outputh, "protected:\n"); - fprintbf(3, outputh, "%s(const IceSpider::Core * core);\n\n", b.first); + fprintbf(3, outputh, "explicit %s(const IceSpider::Core * core);\n\n", b.first); for (const auto & f: b.second->functions) { fprintbf(3, outputh, "%s;\n", f); } fprintbf(1, output, "%s::%s(const IceSpider::Core * core)", b.first, b.first); if (!b.second->proxies.empty()) { - fprintf(output, " :"); + fputs(" :", output); } - fprintf(output, "\n"); + fputs("\n", output); unsigned int pn = 0; for (const auto & p : b.second->proxies) { fprintbf(3, outputh, "const %sPrxPtr prx%u;\n", @@ -353,25 +368,25 @@ namespace IceSpider { fprintbf(3, output, "prx%u(core->getProxy<%s>())", pn, boost::algorithm::replace_all_copy(p, ".", "::")); if (++pn < b.second->proxies.size()) { - fprintf(output, ","); + fputs(",", output); } - fprintf(output, "\n"); + fputs("\n", output); } fprintbf(1, outputh, "}; // %s\n", b.first); fprintbf(1, output, "{ }\n"); } void - RouteCompiler::processRoutes(FILE * output, RouteConfigurationPtr c, const Units & u) const + RouteCompiler::processRoutes(FILE * output, const RouteConfigurationPtr & c, const Units & u) const { - fprintf(output, "\n"); + fputs("\n", output); fprintbf(output, "namespace %s {\n", c->name); fprintbf(1, output, "// Implementation classes.\n\n"); for (const auto & r : c->routes) { processRoute(output, r, u); } fprintbf(output, "} // namespace %s\n\n", c->name); - fprintf(output, "// Register route handlers.\n"); + fputs("// Register route handlers.\n", output); for (const auto & r : c->routes) { fprintbf(output, "FACTORY(%s::%s, IceSpider::RouteHandlerFactory);\n", c->name, r.first); } @@ -386,22 +401,22 @@ namespace IceSpider { fprintbf(1, output, "// path: %s\n", r.second->path); fprintbf(1, output, "class %s : public IceSpider::IRouteHandler", r.first); for (const auto & b : r.second->bases) { - fprintf(output, ",\n"); + fputs(",\n", output); fprintbf(3, output, "public %s", b); } - fprintf(output, " {\n"); + fputs(" {\n", output); fprintbf(2, output, "public:\n"); - fprintbf(3, output, "%s(const IceSpider::Core * core) :\n", r.first); + fprintbf(3, output, "explicit %s(const IceSpider::Core * core) :\n", r.first); fprintbf(4, output, "IceSpider::IRouteHandler(IceSpider::HttpMethod::%s, \"%s\")", methodName, r.second->path); for (const auto & b : r.second->bases) { - fprintf(output, ",\n"); + fputs(",\n", output); fprintbf(4, output, "%s(core)", b); } auto proxies = initializeProxies(output, r.second); for (const auto & p : r.second->params) { if (p.second->hasUserSource) { if (p.second->source == ParameterSource::URL) { - fprintf(output, ",\n"); + fputs(",\n", output); Path path(r.second->path); unsigned int idx = -1; for (const auto & pp : path.parts) { @@ -415,25 +430,22 @@ namespace IceSpider { } else { if (p.second->key) { - fprintf(output, ",\n"); + fputs(",\n", output); fprintbf(4, output, "_pn_%s(\"%s\")", p.first, *p.second->key); } } } if (p.second->defaultExpr) { - fprintf(output, ",\n"); + fputs(",\n", output); fprintbf(4, output, "_pd_%s(%s)", p.first, *p.second->defaultExpr); } } - fprintf(output, "\n"); + fputs("\n", output); fprintbf(3, output, "{\n"); registerOutputSerializers(output, r.second); fprintbf(3, output, "}\n\n"); - fprintbf(3, output, "~%s()\n", r.first); - fprintbf(3, output, "{\n"); - fprintbf(3, output, "}\n\n"); - fprintbf(3, output, "void execute(IceSpider::IHttpRequest * request) const\n"); + fprintbf(3, output, "void execute(IceSpider::IHttpRequest * request) const override\n"); fprintbf(3, output, "{\n"); auto ps = findParameters(r.second, units); bool doneBody = false; @@ -519,7 +531,7 @@ namespace IceSpider { } RouteCompiler::Proxies - RouteCompiler::initializeProxies(FILE * output, RoutePtr r) const + RouteCompiler::initializeProxies(FILE * output, const RoutePtr & r) const { Proxies proxies; int n = 0; @@ -527,7 +539,7 @@ namespace IceSpider { auto proxyName = o.second->operation.substr(0, o.second->operation.find_last_of('.')); if (proxies.find(proxyName) == proxies.end()) { proxies[proxyName] = n; - fprintf(output, ",\n"); + fputs(",\n", output); fprintbf(4, output, "prx%d(core->getProxy<%s>())", n, boost::algorithm::replace_all_copy(proxyName, ".", "::")); n += 1; } @@ -544,7 +556,7 @@ namespace IceSpider { } void - RouteCompiler::addSingleOperation(FILE * output, RoutePtr r, Slice::OperationPtr o) const + RouteCompiler::addSingleOperation(FILE * output, const RoutePtr & r, const Slice::OperationPtr & o) const { auto operation = r->operation->substr(r->operation->find_last_of('.') + 1); if (o->returnType()) { @@ -575,7 +587,7 @@ namespace IceSpider { } void - RouteCompiler::addMashupOperations(FILE * output, RoutePtr r, const Proxies & proxies, const Units & us) const + RouteCompiler::addMashupOperations(FILE * output, const RoutePtr & r, const Proxies & proxies, const Units & us) const { for (const auto & o : r->operations) { auto proxyName = o.second->operation.substr(0, o.second->operation.find_last_of('.')); @@ -624,7 +636,7 @@ namespace IceSpider { if (!isOp) { fprintbf(output, "_p_%s", mi->name()); } - fprintf(output, ";\n"); + fputs(";\n", output); } for (const auto & mutator : r->mutators) { fprintbf(4, output, "%s(request, _responseModel);\n", mutator); diff --git a/icespider/compile/routeCompiler.h b/icespider/compile/routeCompiler.h index 7a2b7ba..fef7648 100644 --- a/icespider/compile/routeCompiler.h +++ b/icespider/compile/routeCompiler.h @@ -17,9 +17,9 @@ namespace IceSpider { RouteCompiler(); RouteConfigurationPtr loadConfiguration(const std::filesystem::path &) const; - Units loadUnits(RouteConfigurationPtr) const; + Units loadUnits(const RouteConfigurationPtr &) const; - void applyDefaults(RouteConfigurationPtr, const Units & u) const; + void applyDefaults(const RouteConfigurationPtr &, const Units & u) const; void compile(const std::filesystem::path & input, const std::filesystem::path & output) const; std::vector searchPath; @@ -28,21 +28,21 @@ namespace IceSpider { typedef std::map Proxies; #pragma GCC visibility push(hidden) - void processConfiguration(FILE * output, FILE * outputh, const std::string & name, RouteConfigurationPtr, const Units &) const; - void processBases(FILE * output, FILE * outputh, RouteConfigurationPtr, const Units &) const; + void processConfiguration(FILE * output, FILE * outputh, const std::string & name, const RouteConfigurationPtr &, const Units &) const; + void processBases(FILE * output, FILE * outputh, const RouteConfigurationPtr &, const Units &) const; void processBase(FILE * output, FILE * outputh, const RouteBases::value_type &, const Units &) const; - void processRoutes(FILE * output, RouteConfigurationPtr, const Units &) const; + void processRoutes(FILE * output, const RouteConfigurationPtr &, const Units &) const; void processRoute(FILE * output, const Routes::value_type &, const Units &) const; - void registerOutputSerializers(FILE * output, RoutePtr) const; - Proxies initializeProxies(FILE * output, RoutePtr) const; + void registerOutputSerializers(FILE * output, const RoutePtr &) const; + Proxies initializeProxies(FILE * output, const RoutePtr &) const; void declareProxies(FILE * output, const Proxies &) const; - void addSingleOperation(FILE * output, RoutePtr, Slice::OperationPtr) const; - void addMashupOperations(FILE * output, RoutePtr, const Proxies &, const Units &) const; - typedef std::map ParameterMap; - static ParameterMap findParameters(RoutePtr, const Units &); + void addSingleOperation(FILE * output, const RoutePtr &, const Slice::OperationPtr &) const; + void addMashupOperations(FILE * output, const RoutePtr &, const Proxies &, const Units &) const; + using ParameterMap = std::map; + static ParameterMap findParameters(const RoutePtr &, const Units &); static Slice::OperationPtr findOperation(const std::string &, const Units &); static Slice::OperationPtr findOperation(const std::string &, const Slice::ContainerPtr &, const Ice::StringSeq & = Ice::StringSeq()); - typedef std::pair Type; + using Type = std::pair; static Type findType(const std::string &, const Units &); static Type findType(const std::string &, const Slice::ContainerPtr &, const Ice::StringSeq & = Ice::StringSeq()); #pragma GCC visibility pop diff --git a/icespider/core/Jamfile.jam b/icespider/core/Jamfile.jam index 91e92e9..15648b8 100644 --- a/icespider/core/Jamfile.jam +++ b/icespider/core/Jamfile.jam @@ -4,7 +4,7 @@ lib stdc++fs ; obj routeOptions : routeOptions.ice : tidy:none ; lib icespider-core : - [ glob *.c *.cpp ] + [ glob *.cpp ] routeOptions : ../common//icespider-common diff --git a/icespider/core/core.cpp b/icespider/core/core.cpp index 50adcab..98ad99c 100644 --- a/icespider/core/core.cpp +++ b/icespider/core/core.cpp @@ -55,7 +55,9 @@ namespace IceSpider { pluginAdapter->destroy(); } - if (communicator) communicator->destroy(); + if (communicator) { + communicator->destroy(); + } } void @@ -103,24 +105,29 @@ namespace IceSpider { defaultErrorReport(request, exception); } + auto demangle(const char * const name) + { + return std::unique_ptr( + __cxxabiv1::__cxa_demangle(name, nullptr, nullptr, nullptr), std::free); + } + AdHocFormatter(LogExp, "Exception type: %?\nDetail: %?\n"); void Core::defaultErrorReport(IHttpRequest * request, const std::exception & exception) const { - char * buf = __cxxabiv1::__cxa_demangle(typeid(exception).name(), NULL, NULL, NULL); + auto buf = demangle(typeid(exception).name()); request->setHeader(H::CONTENT_TYPE, MIME::TEXT_PLAIN); - request->response(500, buf); - LogExp::write(request->getOutputStream(), buf, exception.what()); + request->response(500, buf.get()); + LogExp::write(request->getOutputStream(), buf.get(), exception.what()); request->dump(std::cerr); - LogExp::write(std::cerr, buf, exception.what()); - free(buf); + LogExp::write(std::cerr, buf.get(), exception.what()); } Ice::ObjectPrxPtr Core::getProxy(const char * type) const { - char * buf = __cxxabiv1::__cxa_demangle(type, NULL, NULL, NULL); - char * c = buf; + auto buf = demangle(type); + char * c = buf.get(); int off = 0; while (*c) { if (*(c + 1) == ':' && *c == ':') { @@ -132,8 +139,7 @@ namespace IceSpider { } c += 1; } - auto i = communicator->propertyToProxy(buf); - free(buf); + auto i = communicator->propertyToProxy(buf.get()); return i; } @@ -143,7 +149,9 @@ namespace IceSpider { { auto rpi = r->parts.begin(); for (auto ppi = pathparts.begin(); ppi != pathparts.end(); ++ppi, ++rpi) { - if (!(*rpi)->matches(*ppi)) return false; + if (!(*rpi)->matches(*ppi)) { + return false; + } } return true; } diff --git a/icespider/core/exceptions.h b/icespider/core/exceptions.h index c5076aa..a11ea45 100644 --- a/icespider/core/exceptions.h +++ b/icespider/core/exceptions.h @@ -8,12 +8,12 @@ class DLL_PUBLIC Name : public ::IceSpider::HttpException { \ public: \ Name(); \ - static const int code; \ + static const short code; \ static const std::string message; \ } #define DefineHttpEx(Name, Code, Message) \ Name::Name() : ::IceSpider::HttpException(__FILE__, __LINE__, code, message) { } \ - const int Name::code(Code); \ + const short Name::code(Code); \ const std::string Name::message(Message); namespace IceSpider { diff --git a/icespider/core/hextable.c b/icespider/core/hextable.c deleted file mode 100644 index 3086d49..0000000 --- a/icespider/core/hextable.c +++ /dev/null @@ -1,13 +0,0 @@ -// GCC doesn't support this sort of initialization in C++, only plain C. -// https://gcc.gnu.org/onlinedocs/gcc-5.4.0/gcc/Designated-Inits.html - -const long hextable[] = { - [0 ... '0' - 1] = -1, - ['9' + 1 ... 'A' - 1] = -1, - ['G' ... 'a' - 1] = -1, - ['g' ... 255] = -1, - ['0'] = 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, - ['A'] = 10, 11, 12, 13, 14, 15, - ['a'] = 10, 11, 12, 13, 14, 15 -}; - diff --git a/icespider/core/ihttpRequest.cpp b/icespider/core/ihttpRequest.cpp index b8d866a..13fc447 100644 --- a/icespider/core/ihttpRequest.cpp +++ b/icespider/core/ihttpRequest.cpp @@ -4,8 +4,8 @@ #include "exceptions.h" #include "xwwwFormUrlEncoded.h" #include -#include -#include +#include +#include #include namespace IceSpider { @@ -50,6 +50,7 @@ namespace IceSpider { }; for (auto off = reset(); + // NOLINTNEXTLINE(hicpp-vararg) fscanf(accept.get(), " %n%*[^ /]%n / %n%*[^ ;,]%n", &grpstart, &grpend, &typestart, &typeend) == 0; off = reset()) { if (grpend <= grpstart || typestart <= grpend || typeend <= typestart) { @@ -65,12 +66,14 @@ namespace IceSpider { else if (a->type) { throw Http400_BadRequest(); } + // NOLINTNEXTLINE(hicpp-vararg) if (fscanf(accept.get(), " ; q = %f ", &a->q) == 1) { - if (a->q <= 0.0f || a->q > 1.0f) { + if (a->q <= 0.0F || a->q > 1.0F) { throw Http400_BadRequest(); } } accepts.push_back(a); + // NOLINTNEXTLINE(hicpp-vararg) if (fscanf(accept.get(), " ,") != 0) { break; } @@ -135,23 +138,30 @@ namespace IceSpider { o << '='; XWwwFormUrlEncoded::urlencodeto(o, value.begin(), value.end()); if (e) { - char buf[45]; - struct tm tm; - memset(&tm, 0, sizeof(tm)); + std::string buf(45, 0); + struct tm tm { }; gmtime_r(&*e, &tm); - auto l = strftime(buf, sizeof(buf), "; expires=%a, %d %b %Y %T %Z", &tm); - o.write(buf, l); + buf.resize(strftime(buf.data(), buf.length(), "; expires=%a, %d %b %Y %T %Z", &tm)); + o << buf; + } + if (d) { + "; domain=%?"_fmt(o, *d); + } + if (p) { + "; path=%?"_fmt(o, *p); + } + if (s){ + "; secure"_fmt(o); } - if (d) "; domain=%?"_fmt(o, *d); - if (p) "; path=%?"_fmt(o, *p); - if (s) "; secure"_fmt(o); setHeader(H::SET_COOKIE, o.str()); } template inline std::optional optionalLexicalCast(const OptionalString & p) { - if (p) return wrapLexicalCast(*p); + if (p) { + return wrapLexicalCast(*p); + } return {}; } diff --git a/icespider/core/irouteHandler.cpp b/icespider/core/irouteHandler.cpp index f64658e..244f660 100644 --- a/icespider/core/irouteHandler.cpp +++ b/icespider/core/irouteHandler.cpp @@ -30,10 +30,6 @@ namespace IceSpider { } } - IRouteHandler::~IRouteHandler() - { - } - ContentTypeSerializer IRouteHandler::getSerializer(const AcceptPtr & a, std::ostream & strm) const { @@ -61,7 +57,7 @@ namespace IceSpider { } void - IRouteHandler::addRouteSerializer(const MimeType & ct, StreamSerializerFactoryPtr ssfp) + IRouteHandler::addRouteSerializer(const MimeType & ct, const StreamSerializerFactoryPtr & ssfp) { routeSerializers.erase(ct); routeSerializers.insert({ ct, ssfp }); diff --git a/icespider/core/irouteHandler.h b/icespider/core/irouteHandler.h index ad25ea2..168008e 100644 --- a/icespider/core/irouteHandler.h +++ b/icespider/core/irouteHandler.h @@ -18,7 +18,7 @@ namespace IceSpider { IRouteHandler(HttpMethod, const std::string_view & path); IRouteHandler(HttpMethod, const std::string_view & path, const RouteOptions &); - virtual ~IRouteHandler(); + virtual ~IRouteHandler() = default; virtual void execute(IHttpRequest * request) const = 0; virtual ContentTypeSerializer getSerializer(const AcceptPtr &, std::ostream &) const; @@ -42,7 +42,7 @@ namespace IceSpider { // LCOV_EXCL_STOP } - void addRouteSerializer(const MimeType &, StreamSerializerFactoryPtr); + void addRouteSerializer(const MimeType &, const StreamSerializerFactoryPtr &); }; typedef std::shared_ptr IRouteHandlerPtr; typedef std::shared_ptr IRouteHandlerCPtr; diff --git a/icespider/core/xwwwFormUrlEncoded.cpp b/icespider/core/xwwwFormUrlEncoded.cpp index 7f4bdcd..3de39c3 100644 --- a/icespider/core/xwwwFormUrlEncoded.cpp +++ b/icespider/core/xwwwFormUrlEncoded.cpp @@ -2,11 +2,36 @@ #include "exceptions.h" #include #include +#include namespace ba = boost::algorithm; using namespace std::literals; -extern long hextable[]; +constexpr std::array hextable = []() +{ + std::array hextable{}; + for (int n = 0; n < 255; n++) { + hextable[n] = -1; + } + for (int n = '0'; n <= '9'; n++) { + hextable[n] = n - '0'; + } + for (int n = 'a'; n <= 'f'; n++) { + hextable[n] = hextable[n - 32] = n - 'a' + 10; + } + return hextable; +}(); + +static_assert(hextable['~'] == -1); +static_assert(hextable[' '] == -1); +static_assert(hextable['0'] == 0); +static_assert(hextable['9'] == 9); +static_assert(hextable['a'] == 10); +static_assert(hextable['A'] == 10); +static_assert(hextable['f'] == 15); +static_assert(hextable['F'] == 15); +static_assert(hextable['g'] == -1); +static_assert(hextable['G'] == -1); namespace IceSpider { static const std::string AMP = "&"; @@ -45,15 +70,21 @@ namespace IceSpider { class SetFromString : public Slicer::ValueSource { public: - SetFromString(const std::string & v) : s(v) + explicit SetFromString(const std::string & v) : s(v) { } void set(bool & t) const override { - if (s == TRUE) t = true; - else if (s == FALSE) t = false; - else throw Http400_BadRequest(); + if (s == TRUE) { + t = true; + } + else if (s == FALSE) { + t = false; + } + else { + throw Http400_BadRequest(); + } } void set(std::string & t) const override @@ -62,6 +93,7 @@ namespace IceSpider { } #define SET(T) \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ void set(T & t) const override \ { \ t = boost::lexical_cast(s); \ @@ -74,6 +106,8 @@ namespace IceSpider { SET(Ice::Float); // NOLINTNEXTLINE(clang-analyzer-optin.cplusplus.VirtualCall) SET(Ice::Double); + + private: const std::string & s; }; @@ -83,7 +117,7 @@ namespace IceSpider { return urlencode(s.begin(), s.end()); } - inline char hexchar(char c) { return c < 10 ? '0' + c : 'a' - 10 + c; } + inline auto hexchar(int c) { return c < 10 ? '0' + c : 'a' - 10 + c; } template inline char hexlookup(const T & i) { return hextable[(int)*i]; } @@ -104,8 +138,8 @@ namespace IceSpider { } else if (!isalnum(*i)) { o.put('%'); - o.put(hexchar(*i >> 4)); - o.put(hexchar(*i & 0xf)); + o.put(hexchar(*i >> 4)); // NOLINT(hicpp-signed-bitwise) + o.put(hexchar(*i & 0xf)); // NOLINT(hicpp-signed-bitwise) } else { o.put(*i); @@ -125,7 +159,7 @@ namespace IceSpider { t += ' '; break; case '%': - t += (16 * hexlookup(i + 1)) + hexlookup(i + 2); + t += static_cast((16 * hexlookup(i + 1)) + hexlookup(i + 2)); i += 2; break; default: @@ -165,7 +199,7 @@ namespace IceSpider { } void - XWwwFormUrlEncoded::DeserializeSimple(Slicer::ModelPartPtr mp) + XWwwFormUrlEncoded::DeserializeSimple(const Slicer::ModelPartPtr & mp) { iterateVars([mp](auto &&, const auto && v) { mp->SetValue(SetFromString(v)); @@ -173,7 +207,7 @@ namespace IceSpider { } void - XWwwFormUrlEncoded::DeserializeComplex(Slicer::ModelPartPtr mp) + XWwwFormUrlEncoded::DeserializeComplex(const Slicer::ModelPartPtr & mp) { mp->Create(); iterateVars([mp](auto && k, const auto && v) { @@ -185,7 +219,7 @@ namespace IceSpider { } void - XWwwFormUrlEncoded::DeserializeDictionary(Slicer::ModelPartPtr mp) + XWwwFormUrlEncoded::DeserializeDictionary(const Slicer::ModelPartPtr & mp) { iterateVars([mp](auto && k, const auto && v) { auto p = mp->GetAnonChild(); diff --git a/icespider/core/xwwwFormUrlEncoded.h b/icespider/core/xwwwFormUrlEncoded.h index 855f91c..20f4ce6 100644 --- a/icespider/core/xwwwFormUrlEncoded.h +++ b/icespider/core/xwwwFormUrlEncoded.h @@ -25,9 +25,9 @@ namespace IceSpider { void iterateVars(const KVh & h); - void DeserializeSimple(Slicer::ModelPartPtr mp); - void DeserializeComplex(Slicer::ModelPartPtr mp); - void DeserializeDictionary(Slicer::ModelPartPtr mp); + void DeserializeSimple(const Slicer::ModelPartPtr & mp); + void DeserializeComplex(const Slicer::ModelPartPtr & mp); + void DeserializeDictionary(const Slicer::ModelPartPtr & mp); const std::string input; }; diff --git a/icespider/fileSessions/fileSessions.cpp b/icespider/fileSessions/fileSessions.cpp index ad026da..bee3f7c 100644 --- a/icespider/fileSessions/fileSessions.cpp +++ b/icespider/fileSessions/fileSessions.cpp @@ -17,21 +17,32 @@ namespace IceSpider { class FileSessions : public Plugin, public SessionManager { public: - FileSessions(Ice::CommunicatorPtr c, Ice::PropertiesPtr p) : - ic(c), + FileSessions(Ice::CommunicatorPtr c, const Ice::PropertiesPtr & p) : + ic(std::move(c)), root(p->getProperty("IceSpider.FileSessions.Path")), duration(p->getPropertyAsIntWithDefault("IceSpider.FileSessions.Duration", 3600)) { - if (!root.empty()) - if (!std::filesystem::exists(root)) - std::filesystem::create_directories(root); + if (!root.empty() && !std::filesystem::exists(root)) { + std::filesystem::create_directories(root); + } } - ~FileSessions() + FileSessions(const FileSessions &) = delete; + FileSessions(FileSessions &&) = delete; + + ~FileSessions() override { - removeExpired(); + try { + removeExpired(); + } + catch (...) { + // Meh :) + } } + void operator=(const FileSessions &) = delete; + void operator=(FileSessions &&) = delete; + SessionPtr createSession(const ::Ice::Current &) override { auto s = std::make_shared(); @@ -47,7 +58,7 @@ namespace IceSpider { auto s = load(id); if (s && isExpired(s)) { destroySession(id, current); - return NULL; + return nullptr; } return s; } @@ -68,12 +79,13 @@ namespace IceSpider { } private: - void save(SessionPtr s) + void save(const SessionPtr & s) { - s->lastUsed = time(NULL); + s->lastUsed = time(nullptr); Ice::OutputStream buf(ic); buf.write(s); auto range = buf.finished(); + // NOLINTNEXTLINE(hicpp-signed-bitwise) AdHoc::FileUtils::FileHandle f(root / s->id, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR); sysassert(flock(f.fh, LOCK_EX), -1); sysassert(pwrite(f.fh, range.first, range.second - range.first, 0), -1); @@ -84,7 +96,9 @@ namespace IceSpider { SessionPtr load(const std::string & id) { auto path = root / id; - if (!std::filesystem::exists(path)) return NULL; + if (!std::filesystem::exists(path)) { + return nullptr; + } try { AdHoc::FileUtils::MemMap f(path); sysassert(flock(f.fh, LOCK_SH), -1); @@ -97,7 +111,7 @@ namespace IceSpider { } catch (const AdHoc::SystemException & e) { if (e.errNo == ENOENT) { - return NULL; + return nullptr; } throw; } @@ -105,7 +119,9 @@ namespace IceSpider { void removeExpired() { - if (root.empty() || !std::filesystem::exists(root)) return; + if (root.empty() || !std::filesystem::exists(root)) { + return; + } std::filesystem::directory_iterator di(root); while (di != std::filesystem::directory_iterator()) { auto s = load(di->path()); @@ -116,16 +132,15 @@ namespace IceSpider { } } - bool isExpired(SessionPtr s) + bool isExpired(const SessionPtr & s) { - return (s->lastUsed + s->duration < time(NULL)); + return (s->lastUsed + s->duration < time(nullptr)); } template R sysassert(R rtn, ER ertn) { if (rtn == ertn) { - fprintf(stderr, "%s\n", strerror(errno)); throw SessionError(strerror(errno)); } return rtn; diff --git a/icespider/testing/testRequest.cpp b/icespider/testing/testRequest.cpp index 5aa5bce..b866308 100644 --- a/icespider/testing/testRequest.cpp +++ b/icespider/testing/testRequest.cpp @@ -96,10 +96,12 @@ namespace IceSpider { void TestRequest::set(const std::string_view & key, const OptionalString & val, MapVars & vars) { - if (val) + if (val) { vars[std::string(key)] = *val; - else + } + else { vars.erase(vars.find(key)); + } } std::istream & @@ -137,12 +139,13 @@ namespace IceSpider { { if (responseHeaders.empty()) { while (true) { - char buf[BUFSIZ], n[BUFSIZ], v[BUFSIZ]; - output.getline(buf, BUFSIZ); - if (sscanf(buf, "%[^:]: %[^\r]", n, v) != 2) { + std::array buf {}, n {}, v {}; + output.getline(buf.data(), BUFSIZ); + // NOLINTNEXTLINE(hicpp-vararg) + if (sscanf(buf.data(), "%[^:]: %[^\r]", n.data(), v.data()) != 2) { break; } - responseHeaders[n] = v; + responseHeaders[n.data()] = v.data(); } } return responseHeaders; diff --git a/icespider/unittests/Jamfile.jam b/icespider/unittests/Jamfile.jam index bf31020..ba44d29 100644 --- a/icespider/unittests/Jamfile.jam +++ b/icespider/unittests/Jamfile.jam @@ -26,13 +26,11 @@ lib stdc++fs ; lib dl ; path-constant me : . ; -alias testCommon : : - ROOT=\"$(me)\" - ..//Ice - : : +alias testCommon : : : : ..//Ice boost_utf ROOT=\"$(me)\" + tidy:hicpp-vararg ; run diff --git a/icespider/unittests/testApp.cpp b/icespider/unittests/testApp.cpp index cd43674..cabcd0c 100644 --- a/icespider/unittests/testApp.cpp +++ b/icespider/unittests/testApp.cpp @@ -176,10 +176,11 @@ class TestSerice : public TestIceSpider::TestApi { std::string simplei(Ice::Int n, const Ice::Current &) override { - return boost::lexical_cast(n); + return std::to_string(n); } }; +// NOLINTNEXTLINE(hicpp-special-member-functions) class TestApp : public CoreWithDefaultRouter { public: TestApp() : @@ -189,18 +190,19 @@ class TestApp : public CoreWithDefaultRouter { adp->add(std::make_shared(), Ice::stringToIdentity("Test")); } - ~TestApp() + ~TestApp() override { adp->deactivate(); adp->destroy(); } + private: Ice::ObjectAdapterPtr adp; }; class Dummy : public IceSpider::Plugin, TestIceSpider::DummyPlugin { public: - Dummy(Ice::CommunicatorPtr, Ice::PropertiesPtr) { } + Dummy(const Ice::CommunicatorPtr &, const Ice::PropertiesPtr &) { } }; NAMEDFACTORY("DummyPlugin", Dummy, IceSpider::PluginFactory); @@ -295,7 +297,7 @@ BOOST_AUTO_TEST_CASE( testCallPost1234 ) { TestRequest requestUpdateItem(this, HttpMethod::POST, "/1234"); requestUpdateItem.env["CONTENT_TYPE"] = "application/json"; - requestUpdateItem.input << "{\"value\": \"some value\"}"; + requestUpdateItem.input << R"({"value": "some value"})"; process(&requestUpdateItem); auto h = requestUpdateItem.getResponseHeaders(); BOOST_REQUIRE_EQUAL(h["Status"], "200 OK"); @@ -306,7 +308,7 @@ BOOST_AUTO_TEST_CASE( testCallPost1234 ) BOOST_AUTO_TEST_CASE( testCallPost1234NoContentType ) { TestRequest requestUpdateItem(this, HttpMethod::POST, "/1234"); - requestUpdateItem.input << "{\"value\": \"some value\"}"; + requestUpdateItem.input << R"({"value": "some value"})"; process(&requestUpdateItem); auto h = requestUpdateItem.getResponseHeaders(); BOOST_REQUIRE_EQUAL(h["Status"], "400 Bad Request"); @@ -318,7 +320,7 @@ BOOST_AUTO_TEST_CASE( testCallPost1234UnsupportedMediaType ) { TestRequest requestUpdateItem(this, HttpMethod::POST, "/1234"); requestUpdateItem.env["CONTENT_TYPE"] = "application/notathing"; - requestUpdateItem.input << "value=\"some value\""; + requestUpdateItem.input << R"(value="some value")"; process(&requestUpdateItem); auto h = requestUpdateItem.getResponseHeaders(); BOOST_REQUIRE_EQUAL(h["Status"], "415 Unsupported Media Type"); @@ -498,7 +500,7 @@ BOOST_AUTO_TEST_CASE( testCookies ) class DummyErrorHandler : public IceSpider::ErrorHandler { public: IceSpider::ErrorHandlerResult - handleError(IceSpider::IHttpRequest * request, const std::exception & ex) const + handleError(IceSpider::IHttpRequest * request, const std::exception & ex) const override { if (const auto * tex = dynamic_cast(&ex)) { if (tex->message == "404") { diff --git a/icespider/unittests/testCompile.cpp b/icespider/unittests/testCompile.cpp index 3677207..0135303 100644 --- a/icespider/unittests/testCompile.cpp +++ b/icespider/unittests/testCompile.cpp @@ -23,7 +23,8 @@ class CoreFixture { modeDir(binDir.lexically_relative(rootDir / "bin" / "testCompile.test")) { } - std::filesystem::path modeDir; + // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes) + const std::filesystem::path modeDir; }; namespace std { diff --git a/icespider/unittests/testFcgi.cpp b/icespider/unittests/testFcgi.cpp index b64dd47..d543678 100644 --- a/icespider/unittests/testFcgi.cpp +++ b/icespider/unittests/testFcgi.cpp @@ -11,7 +11,9 @@ using namespace std::literals; namespace std { template ostream & operator<<(ostream & s, const std::optional & o) { - if (o) s << *o; + if (o) { + s << *o; + } return s; } } @@ -42,6 +44,8 @@ class TestRequest : public IceSpider::CgiRequestBase { return std::cin; } // LCOV_EXCL_STOP + + // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes) mutable std::stringstream out; }; @@ -63,28 +67,31 @@ class TestPayloadRequest : public TestRequest { std::istream & in; }; +// NOLINTNEXTLINE(hicpp-special-member-functions) class CharPtrPtrArray : public std::vector { public: CharPtrPtrArray() { - push_back(NULL); + push_back(nullptr); } - CharPtrPtrArray(const std::vector & a) + explicit CharPtrPtrArray(const std::vector & a) { for (const auto & e : a) { push_back(strdup(e.c_str())); } - push_back(NULL); + push_back(nullptr); } ~CharPtrPtrArray() { for (const auto & e : *this) { + // NOLINTNEXTLINE(hicpp-no-malloc) free(e); } } + // NOLINTNEXTLINE(hicpp-explicit-conversions) operator char **() { return &front(); @@ -281,7 +288,7 @@ BOOST_AUTO_TEST_CASE( postxwwwformurlencoded_complex ) BOOST_AUTO_TEST_CASE( postjson_complex ) { CharPtrPtrArray env ({ "SCRIPT_NAME=/", "REQUEST_METHOD=No", "CONTENT_TYPE=application/json" }); - std::stringstream f("{\"alpha\":\"abcde\",\"number\":3.14,\"boolean\":true,\"empty\":\"\",\"spaces\":\"This is a string.\"}"); + std::stringstream f(R"J({"alpha":"abcde","number":3.14,"boolean":true,"empty":"","spaces":"This is a string."})J"); TestPayloadRequest r(this, env, f); auto n = *r.getBody(); BOOST_REQUIRE_EQUAL("abcde", n->alpha); @@ -294,7 +301,7 @@ BOOST_AUTO_TEST_CASE( postjson_complex ) BOOST_AUTO_TEST_CASE( postjson_dictionary ) { CharPtrPtrArray env ({ "SCRIPT_NAME=/", "REQUEST_METHOD=No", "CONTENT_TYPE=application/json" }); - std::stringstream f("{\"alpha\":\"abcde\",\"number\":\"3.14\",\"boolean\":\"true\",\"empty\":\"\",\"spaces\":\"This is a string.\"}"); + std::stringstream f(R"J({"alpha":"abcde","number":"3.14","boolean":"true","empty":"","spaces":"This is a string."})J"); TestPayloadRequest r(this, env, f); auto n = *r.getBody(); BOOST_REQUIRE_EQUAL(5, n.size()); diff --git a/icespider/unittests/testFileSessions.cpp b/icespider/unittests/testFileSessions.cpp index 69e4d01..1ab75f7 100644 --- a/icespider/unittests/testFileSessions.cpp +++ b/icespider/unittests/testFileSessions.cpp @@ -20,6 +20,7 @@ class TestCore : public IceSpider::CoreWithDefaultRouter { root(communicator->getProperties()->getProperty("IceSpider.FileSessions.Path")) { } + // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes) const std::filesystem::path root; }; @@ -46,7 +47,7 @@ BOOST_AUTO_TEST_CASE( createAndDestroy ) auto s = prx->createSession(); BOOST_REQUIRE(std::filesystem::exists(root / s->id)); BOOST_REQUIRE_EQUAL(0, s->duration); - BOOST_REQUIRE_EQUAL(time(NULL), s->lastUsed); + BOOST_REQUIRE_EQUAL(time(nullptr), s->lastUsed); prx->destroySession(s->id); BOOST_REQUIRE(!std::filesystem::exists(root / s->id)); } @@ -71,7 +72,7 @@ BOOST_AUTO_TEST_CASE( createAndExpire ) auto s = prx->createSession(); BOOST_REQUIRE(std::filesystem::exists(root / s->id)); BOOST_REQUIRE_EQUAL(0, s->duration); - BOOST_REQUIRE_EQUAL(time(NULL), s->lastUsed); + BOOST_REQUIRE_EQUAL(time(nullptr), s->lastUsed); usleep(1001000); BOOST_REQUIRE(std::filesystem::exists(root / s->id)); BOOST_REQUIRE(!prx->getSession(s->id)); diff --git a/icespider/xslt/xsltStreamSerializer.cpp b/icespider/xslt/xsltStreamSerializer.cpp index 72dead1..d798835 100644 --- a/icespider/xslt/xsltStreamSerializer.cpp +++ b/icespider/xslt/xsltStreamSerializer.cpp @@ -67,7 +67,7 @@ namespace IceSpider { if (!result) { throw xmlpp::exception("Failed to apply XSL transform"); } - xmlOutputBufferPtr buf = xmlOutputBufferCreateIO(xmlstrmwritecallback, xmlstrmclosecallback, &strm, NULL); + xmlOutputBufferPtr buf = xmlOutputBufferCreateIO(xmlstrmwritecallback, xmlstrmclosecallback, &strm, nullptr); if (xmlStrcmp(stylesheet->method, BAD_CAST "html") == 0) { htmlDocContentDumpFormatOutput(buf, result, (const char *) stylesheet->encoding, 0); } -- cgit v1.2.3