bmcweb: Fix a bunch of warnings

bmcweb classically has not taken a strong opinion on warnings.  With
this commit, that policy is changing, and bmcweb will invoke the best
warnings we are able to enable, and turn on -Werror for all builds.

This is intended to reduce the likelihood of hard-to-debug situations
that the compiler coulve caught early on.

Change-Id: I57474410821e82666b3a108cfd0db7d070e8900a
Signed-off-by: Ed Tanous <ed@tanous.net>

8 files changed, 121 insertions, 98 deletions

diff --git a/crow/include/crow/http_connection.h b/crow/include/crow/http_connection.h
index c02f4ec1e3..d7598bd168 100644
--- a/crow/include/crow/http_connection.h
+++ b/crow/include/crow/http_connection.h
@@ -345,7 +345,7 @@ class Connection
             };
 
             ctx = detail::Context<Middlewares...>();
-            req->middlewareContext = (void*)&ctx;
+            req->middlewareContext = static_cast<void*>(&ctx);
             req->ioService = &adaptor.get_executor().context();
             detail::middlewareCallHelper<
                 0, decltype(ctx), decltype(*middlewares), Middlewares...>(
@@ -388,14 +388,13 @@ class Connection
             needToCallAfterHandlers = false;
 
             // call all afterHandler of middlewares
-            detail::afterHandlersCallHelper<((int)sizeof...(Middlewares) - 1),
-                                            decltype(ctx),
-                                            decltype(*middlewares)>(
-                *middlewares, ctx, *req, res);
+            detail::afterHandlersCallHelper<
+                static_cast<int>(sizeof...(Middlewares) - 1), decltype(ctx),
+                decltype(*middlewares)>(*middlewares, ctx, *req, res);
         }
 
         // auto self = this->shared_from_this();
-        res.completeRequestHandler = res.completeRequestHandler = [] {};
+        res.completeRequestHandler = [] {};
 
         if (!adaptor.lowest_layer().is_open())
         {
diff --git a/crow/include/crow/http_request.h b/crow/include/crow/http_request.h
index 0ba02664de..fa12c26e92 100644
--- a/crow/include/crow/http_request.h
+++ b/crow/include/crow/http_request.h
@@ -27,7 +27,7 @@ struct Request
     {
     }
 
-    const boost::beast::http::verb method() const
+    boost::beast::http::verb method() const
     {
         return req.method();
     }
diff --git a/crow/include/crow/http_server.h b/crow/include/crow/http_server.h
index 75b49a463f..eace64fcfe 100644
--- a/crow/include/crow/http_server.h
+++ b/crow/include/crow/http_server.h
@@ -121,17 +121,17 @@ class Server
         boost::asio::deadline_timer timer(*ioService);
         timer.expires_from_now(boost::posix_time::seconds(1));
 
-        std::function<void(const boost::system::error_code& ec)> handler;
-        handler = [&](const boost::system::error_code& ec) {
+        std::function<void(const boost::system::error_code& ec)> fHandler;
+        fHandler = [&](const boost::system::error_code& ec) {
             if (ec)
             {
                 return;
             }
             timerQueue.process();
             timer.expires_from_now(boost::posix_time::seconds(1));
-            timer.async_wait(handler);
+            timer.async_wait(fHandler);
         };
-        timer.async_wait(handler);
+        timer.async_wait(fHandler);
 
         if (tickFunction && tickInterval.count() > 0)
         {
diff --git a/crow/include/crow/logging.h b/crow/include/crow/logging.h
index 9da47b733a..0d24940820 100644
--- a/crow/include/crow/logging.h
+++ b/crow/include/crow/logging.h
@@ -30,6 +30,7 @@ class ILogHandler
 {
   public:
     virtual void log(std::string message, LogLevel level) = 0;
+    virtual ~ILogHandler(){};
 };
 
 class CerrLogHandler : public ILogHandler
diff --git a/crow/include/crow/query_string.h b/crow/include/crow/query_string.h
index 553960eb6f..b38d0ed1be 100644
--- a/crow/include/crow/query_string.h
+++ b/crow/include/crow/query_string.h
@@ -19,16 +19,16 @@ int qsStrncmp(const char* s, const char* qs, size_t n);
  *  Also decodes the value portion of the k/v pair *in-place*.  In a future
  *  enhancement it will also have a compile-time option of sorting qs_kv
  *  alphabetically by key.  */
-int qsParse(char* qs, char* qs_kv[], int qs_kv_size);
+size_t qsParse(char* qs, char* qs_kv[], size_t qs_kv_size);
 
 /*  Used by qs_parse to decode the value portion of a k/v pair  */
-int qsDecode(char* qs);
+size_t qsDecode(char* qs);
 
 /*  Looks up the value according to the key on a pre-processed query string
  *  A future enhancement will be a compile-time option to look up the key
  *  in a pre-sorted qs_kv array via a binary search.  */
 // char * qs_k2v(const char * key, char * qs_kv[], int qs_kv_size);
-char* qsK2v(const char* key, char* const* qs_kv, int qs_kv_size, int nth);
+char* qsK2v(const char* key, char* const* qs_kv, size_t qs_kv_size, size_t nth);
 
 /*  Non-destructive lookup of value, based on key.  User provides the
  *  destinaton string and length.  */
@@ -127,9 +127,9 @@ inline int qsStrncmp(const char* s, const char* qs, size_t n)
     }
 }
 
-inline int qsParse(char* qs, char* qs_kv[], int qs_kv_size)
+inline size_t qsParse(char* qs, char* qs_kv[], size_t qs_kv_size)
 {
-    int i, j;
+    size_t i, j;
     char* substrPtr;
 
     for (i = 0; i < qs_kv_size; i++)
@@ -184,9 +184,9 @@ inline int qsParse(char* qs, char* qs_kv[], int qs_kv_size)
     return i;
 }
 
-inline int qsDecode(char* qs)
+inline size_t qsDecode(char* qs)
 {
-    int i = 0, j = 0;
+    size_t i = 0, j = 0;
 
     while (BMCWEB_QS_ISQSCHR(qs[j]))
     {
@@ -217,10 +217,10 @@ inline int qsDecode(char* qs)
     return i;
 }
 
-inline char* qsK2v(const char* key, char* const* qs_kv, int qs_kv_size,
-                   int nth = 0)
+inline char* qsK2v(const char* key, char* const* qs_kv, size_t qs_kv_size,
+                   size_t nth = 0)
 {
-    int i;
+    size_t i;
     size_t keyLen, skip;
 
     keyLen = strlen(key);
@@ -353,7 +353,8 @@ class QueryString
 
         keyValuePairs.resize(maxKeyValuePairsCount);
 
-        int count = qsParse(&url[0], &keyValuePairs[0], maxKeyValuePairsCount);
+        size_t count =
+            qsParse(&url[0], &keyValuePairs[0], maxKeyValuePairsCount);
         keyValuePairs.resize(count);
     }
 
@@ -391,7 +392,7 @@ class QueryString
         std::string plus = name + "[]";
         char* element = nullptr;
 
-        int count = 0;
+        size_t count = 0;
         while (1)
         {
             element = qsK2v(plus.c_str(), keyValuePairs.data(),
diff --git a/crow/include/crow/routing.h b/crow/include/crow/routing.h
index 746e115894..d7e3fd33f0 100644
--- a/crow/include/crow/routing.h
+++ b/crow/include/crow/routing.h
@@ -63,7 +63,8 @@ class BaseRule
     }
 
   protected:
-    uint32_t methodsBitfield{1 << (int)boost::beast::http::verb::get};
+    uint32_t methodsBitfield{
+        1 << static_cast<int>(boost::beast::http::verb::get)};
 
     std::string rule;
     std::string nameStr;
@@ -362,29 +363,33 @@ template <typename T> struct RuleParameterTraits
     using self_t = T;
     WebSocketRule& websocket()
     {
-        auto p = new WebSocketRule(((self_t*)this)->rule);
-        ((self_t*)this)->ruleToUpgrade.reset(p);
+        self_t* s = static_cast<self_t*>(this);
+        auto p = new WebSocketRule(s->rule);
+        s->ruleToUpgrade.reset(p);
         return *p;
     }
 
     self_t& name(std::string name) noexcept
     {
-        ((self_t*)this)->nameStr = std::move(name);
-        return (self_t&)*this;
+        self_t* s = static_cast<self_t*>(this);
+        s->nameStr = std::move(name);
+        return *s;
     }
 
     self_t& methods(boost::beast::http::verb method)
     {
-        ((self_t*)this)->methodsBitfield = 1 << (int)method;
-        return (self_t&)*this;
+        self_t* s = static_cast<self_t*>(this);
+        s->methodsBitfield = 1 << static_cast<int>(method);
+        return *s;
     }
 
     template <typename... MethodArgs>
     self_t& methods(boost::beast::http::verb method, MethodArgs... args_method)
     {
+        self_t* s = static_cast<self_t*>(this);
         methods(args_method...);
-        ((self_t*)this)->methodsBitfield |= 1 << (int)method;
-        return (self_t&)*this;
+        s->methodsBitfield |= 1 << static_cast<int>(method);
+        return *s;
     }
 };
 
@@ -581,7 +586,7 @@ class Trie
     struct Node
     {
         unsigned ruleIndex{};
-        std::array<unsigned, (int)ParamType::MAX> paramChildrens{};
+        std::array<unsigned, static_cast<int>(ParamType::MAX)> paramChildrens{};
         boost::container::flat_map<std::string, unsigned> children;
 
         bool isSimpleNode() const
@@ -675,14 +680,15 @@ class Trie
                     route_indexes.push_back(child->ruleIndex);
                 }
                 findRouteIndexes(req_url, route_indexes, child,
-                                 pos + fragment.size());
+                                 pos + static_cast<unsigned>(fragment.size()));
             }
             else
             {
                 if (req_url.compare(pos, fragment.size(), fragment) == 0)
                 {
-                    findRouteIndexes(req_url, route_indexes, child,
-                                     pos + fragment.size());
+                    findRouteIndexes(
+                        req_url, route_indexes, child,
+                        pos + static_cast<unsigned>(fragment.size()));
                 }
             }
         }
@@ -713,7 +719,7 @@ class Trie
                 }
             };
 
-        if (node->paramChildrens[(int)ParamType::INT])
+        if (node->paramChildrens[static_cast<int>(ParamType::INT)])
         {
             char c = req_url[pos];
             if ((c >= '0' && c <= '9') || c == '+' || c == '-')
@@ -725,17 +731,18 @@ class Trie
                 if (errno != ERANGE && eptr != req_url.data() + pos)
                 {
                     params->intParams.push_back(value);
-                    auto ret =
-                        find(req_url,
-                             &nodes[node->paramChildrens[(int)ParamType::INT]],
-                             eptr - req_url.data(), params);
+                    auto ret = find(
+                        req_url,
+                        &nodes[node->paramChildrens[static_cast<int>(
+                            ParamType::INT)]],
+                        static_cast<unsigned>(eptr - req_url.data()), params);
                     updateFound(ret);
                     params->intParams.pop_back();
                 }
             }
         }
 
-        if (node->paramChildrens[(int)ParamType::UINT])
+        if (node->paramChildrens[static_cast<int>(ParamType::UINT)])
         {
             char c = req_url[pos];
             if ((c >= '0' && c <= '9') || c == '+')
@@ -747,17 +754,18 @@ class Trie
                 if (errno != ERANGE && eptr != req_url.data() + pos)
                 {
                     params->uintParams.push_back(value);
-                    auto ret =
-                        find(req_url,
-                             &nodes[node->paramChildrens[(int)ParamType::UINT]],
-                             eptr - req_url.data(), params);
+                    auto ret = find(
+                        req_url,
+                        &nodes[node->paramChildrens[static_cast<int>(
+                            ParamType::UINT)]],
+                        static_cast<unsigned>(eptr - req_url.data()), params);
                     updateFound(ret);
                     params->uintParams.pop_back();
                 }
             }
         }
 
-        if (node->paramChildrens[(int)ParamType::DOUBLE])
+        if (node->paramChildrens[static_cast<int>(ParamType::DOUBLE)])
         {
             char c = req_url[pos];
             if ((c >= '0' && c <= '9') || c == '+' || c == '-' || c == '.')
@@ -770,17 +778,18 @@ class Trie
                     params->doubleParams.push_back(value);
                     auto ret = find(
                         req_url,
-                        &nodes[node->paramChildrens[(int)ParamType::DOUBLE]],
-                        eptr - req_url.data(), params);
+                        &nodes[node->paramChildrens[static_cast<int>(
+                            ParamType::DOUBLE)]],
+                        static_cast<unsigned>(eptr - req_url.data()), params);
                     updateFound(ret);
                     params->doubleParams.pop_back();
                 }
             }
         }
 
-        if (node->paramChildrens[(int)ParamType::STRING])
+        if (node->paramChildrens[static_cast<int>(ParamType::STRING)])
         {
-            size_t epos = pos;
+            unsigned epos = pos;
             for (; epos < req_url.size(); epos++)
             {
                 if (req_url[epos] == '/')
@@ -791,26 +800,27 @@ class Trie
             {
                 params->stringParams.emplace_back(
                     req_url.substr(pos, epos - pos));
-                auto ret =
-                    find(req_url,
-                         &nodes[node->paramChildrens[(int)ParamType::STRING]],
-                         epos, params);
+                auto ret = find(req_url,
+                                &nodes[node->paramChildrens[static_cast<int>(
+                                    ParamType::STRING)]],
+                                epos, params);
                 updateFound(ret);
                 params->stringParams.pop_back();
             }
         }
 
-        if (node->paramChildrens[(int)ParamType::PATH])
+        if (node->paramChildrens[static_cast<int>(ParamType::PATH)])
         {
-            size_t epos = req_url.size();
+            unsigned epos = static_cast<unsigned>(req_url.size());
 
             if (epos != pos)
             {
                 params->stringParams.emplace_back(
                     req_url.substr(pos, epos - pos));
-                auto ret = find(
-                    req_url, &nodes[node->paramChildrens[(int)ParamType::PATH]],
-                    epos, params);
+                auto ret = find(req_url,
+                                &nodes[node->paramChildrens[static_cast<int>(
+                                    ParamType::PATH)]],
+                                epos, params);
                 updateFound(ret);
                 params->stringParams.pop_back();
             }
@@ -823,7 +833,9 @@ class Trie
 
             if (req_url.compare(pos, fragment.size(), fragment) == 0)
             {
-                auto ret = find(req_url, child, pos + fragment.size(), params);
+                auto ret =
+                    find(req_url, child,
+                         pos + static_cast<unsigned>(fragment.size()), params);
                 updateFound(ret);
             }
         }
@@ -858,12 +870,16 @@ class Trie
                 {
                     if (url.compare(i, x.name.size(), x.name) == 0)
                     {
-                        if (!nodes[idx].paramChildrens[(int)x.type])
+                        if (!nodes[idx]
+                                 .paramChildrens[static_cast<size_t>(x.type)])
                         {
                             auto newNodeIdx = newNode();
-                            nodes[idx].paramChildrens[(int)x.type] = newNodeIdx;
+                            nodes[idx]
+                                .paramChildrens[static_cast<size_t>(x.type)] =
+                                newNodeIdx;
                         }
-                        idx = nodes[idx].paramChildrens[(int)x.type];
+                        idx = nodes[idx]
+                                  .paramChildrens[static_cast<size_t>(x.type)];
                         i += x.name.size();
                         break;
                     }
@@ -888,15 +904,15 @@ class Trie
     }
 
   private:
-    void debugNodePrint(Node* n, int level)
+    void debugNodePrint(Node* n, unsigned level)
     {
-        for (int i = 0; i < (int)ParamType::MAX; i++)
+        for (size_t i = 0; i < static_cast<size_t>(ParamType::MAX); i++)
         {
             if (n->paramChildrens[i])
             {
                 BMCWEB_LOG_DEBUG << std::string(
                     2 * level, ' ') /*<< "("<<n->paramChildrens[i]<<") "*/;
-                switch ((ParamType)i)
+                switch (static_cast<ParamType>(i))
                 {
                     case ParamType::INT:
                         BMCWEB_LOG_DEBUG << "<int>";
@@ -950,7 +966,7 @@ class Trie
     unsigned newNode()
     {
         nodes.resize(nodes.size() + 1);
-        return nodes.size() - 1;
+        return static_cast<unsigned>(nodes.size() - 1);
     }
 
     std::vector<Node> nodes;
@@ -991,13 +1007,14 @@ class Router
                                std::unique_ptr<BaseRule> ruleObject)
     {
         rules.emplace_back(std::move(ruleObject));
-        trie.add(rule, rules.size() - 1);
+        trie.add(rule, static_cast<unsigned>(rules.size() - 1));
 
         // directory case:
         //   request to `/about' url matches `/about/' rule
         if (rule.size() > 2 && rule.back() == '/')
         {
-            trie.add(rule.substr(0, rule.size() - 1), rules.size() - 1);
+            trie.add(rule.substr(0, rule.size() - 1),
+                     static_cast<unsigned>(rules.size() - 1));
         }
     }
 
@@ -1056,12 +1073,12 @@ class Router
             return;
         }
 
-        if ((rules[ruleIndex]->getMethods() & (1 << (uint32_t)req.method())) ==
-            0)
+        if ((rules[ruleIndex]->getMethods() &
+             (1 << static_cast<uint32_t>(req.method()))) == 0)
         {
             BMCWEB_LOG_DEBUG << "Rule found but method mismatch: " << req.url
                              << " with " << req.methodString() << "("
-                             << (uint32_t)req.method() << ") / "
+                             << static_cast<uint32_t>(req.method()) << ") / "
                              << rules[ruleIndex]->getMethods();
             res = Response(boost::beast::http::status::not_found);
             res.end();
@@ -1069,7 +1086,7 @@ class Router
         }
 
         BMCWEB_LOG_DEBUG << "Matched rule (upgrade) '" << rules[ruleIndex]->rule
-                         << "' " << (uint32_t)req.method() << " / "
+                         << "' " << static_cast<uint32_t>(req.method()) << " / "
                          << rules[ruleIndex]->getMethods();
 
         // any uncaught exceptions become 500s
@@ -1134,12 +1151,12 @@ class Router
             return;
         }
 
-        if ((rules[ruleIndex]->getMethods() & (1 << (uint32_t)req.method())) ==
-            0)
+        if ((rules[ruleIndex]->getMethods() &
+             (1 << static_cast<uint32_t>(req.method()))) == 0)
         {
             BMCWEB_LOG_DEBUG << "Rule found but method mismatch: " << req.url
                              << " with " << req.methodString() << "("
-                             << (uint32_t)req.method() << ") / "
+                             << static_cast<uint32_t>(req.method()) << ") / "
                              << rules[ruleIndex]->getMethods();
             res = Response(boost::beast::http::status::not_found);
             res.end();
@@ -1147,7 +1164,7 @@ class Router
         }
 
         BMCWEB_LOG_DEBUG << "Matched rule '" << rules[ruleIndex]->rule << "' "
-                         << (uint32_t)req.method() << " / "
+                         << static_cast<uint32_t>(req.method()) << " / "
                          << rules[ruleIndex]->getMethods();
 
         // any uncaught exceptions become 500s
diff --git a/crow/include/crow/timer_queue.h b/crow/include/crow/timer_queue.h
index bf1e084a00..556fdb3c30 100644
--- a/crow/include/crow/timer_queue.h
+++ b/crow/include/crow/timer_queue.h
@@ -22,10 +22,11 @@ class TimerQueue
 
     void cancel(int k)
     {
-        unsigned int index = static_cast<unsigned int>(k - step);
-        if (index < dq.size())
+        int index = k - step;
+
+        if (index < static_cast<int>(dq.size()))
         {
-            dq[index].second = nullptr;
+            dq[static_cast<size_t>(index)].second = nullptr;
         }
     }
 
@@ -33,7 +34,7 @@ class TimerQueue
     {
         dq.push_back(
             std::make_pair(std::chrono::steady_clock::now(), std::move(f)));
-        int ret = step + dq.size() - 1;
+        int ret = step + static_cast<int>(dq.size()) - 1;
 
         BMCWEB_LOG_DEBUG << "timer add inside: " << this << ' ' << ret;
         return ret;
diff --git a/crow/include/crow/utility.h b/crow/include/crow/utility.h
index a07c0415af..5c7a7f6265 100644
--- a/crow/include/crow/utility.h
+++ b/crow/include/crow/utility.h
@@ -524,7 +524,7 @@ struct function_traits<std::function<r(Args...)>>
 };
 
 inline static std::string base64encode(
-    const char* data, size_t size,
+    const uint8_t* data, size_t size,
     const char* key =
         "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/")
 {
@@ -533,36 +533,36 @@ inline static std::string base64encode(
     auto it = ret.begin();
     while (size >= 3)
     {
-        *it++ = key[(((unsigned char)*data) & 0xFC) >> 2];
-        unsigned char h = (((unsigned char)*data++) & 0x03) << 4;
-        *it++ = key[h | ((((unsigned char)*data) & 0xF0) >> 4)];
-        h = (((unsigned char)*data++) & 0x0F) << 2;
-        *it++ = key[h | ((((unsigned char)*data) & 0xC0) >> 6)];
-        *it++ = key[((unsigned char)*data++) & 0x3F];
+        *it++ = key[(*data & 0xFC) >> 2];
+        uint8_t h = static_cast<uint8_t>((*data++ & 0x03) << 4);
+        *it++ = key[h | ((*data & 0xF0) >> 4)];
+        h = static_cast<uint8_t>((*data++ & 0x0F) << 2);
+        *it++ = key[h | ((*data & 0xC0) >> 6)];
+        *it++ = key[*data++ & 0x3F];
 
         size -= 3;
     }
     if (size == 1)
     {
-        *it++ = key[(((unsigned char)*data) & 0xFC) >> 2];
-        unsigned char h = (((unsigned char)*data++) & 0x03) << 4;
+        *it++ = key[(*data & 0xFC) >> 2];
+        uint8_t h = static_cast<uint8_t>((*data++ & 0x03) << 4);
         *it++ = key[h];
         *it++ = '=';
         *it++ = '=';
     }
     else if (size == 2)
     {
-        *it++ = key[(((unsigned char)*data) & 0xFC) >> 2];
-        unsigned char h = (((unsigned char)*data++) & 0x03) << 4;
-        *it++ = key[h | ((((unsigned char)*data) & 0xF0) >> 4)];
-        h = (((unsigned char)*data++) & 0x0F) << 2;
+        *it++ = key[(*data & 0xFC) >> 2];
+        uint8_t h = static_cast<uint8_t>((*data++ & 0x03) << 4);
+        *it++ = key[h | ((*data & 0xF0) >> 4)];
+        h = static_cast<uint8_t>((*data++ & 0x0F) << 2);
         *it++ = key[h];
         *it++ = '=';
     }
     return ret;
 }
 
-inline static std::string base64encodeUrlsafe(const char* data, size_t size)
+inline static std::string base64encodeUrlsafe(const uint8_t* data, size_t size)
 {
     return base64encode(
         data, size,
@@ -669,10 +669,14 @@ inline bool base64Decode(const std::string_view input, std::string& output)
 
 inline void escapeHtml(std::string& data)
 {
+    if (data.empty())
+    {
+        return;
+    }
     std::string buffer;
-    // less than 5% of characters should be larger, so reserve a buffer of the
+    // less than 10% of characters should be larger, so reserve a buffer of the
     // right size
-    buffer.reserve(data.size() * 1.05);
+    buffer.reserve(data.size() * 11 / 10);
     for (size_t pos = 0; pos != data.size(); ++pos)
     {
         switch (data[pos])