Fix a bunch of warnings
using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100
Seems like a good place to start, and would improve things a bit
type-wise. This patchset attempts to correct all the issues in one
shot.
Tested:
It builds. Will test various subsystems that have been touched
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I588c26440e5a97f718a0f0ea74cc84107d53aa1e
diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp
index 1b82697..8c46cf4 100644
--- a/include/dbus_monitor.hpp
+++ b/include/dbus_monitor.hpp
@@ -109,7 +109,7 @@
connection->sendText(j.dump());
return 0;
-};
+}
template <typename... Middlewares> void requestRoutes(Crow<Middlewares...>& app)
{
@@ -150,7 +150,7 @@
nlohmann::json::iterator paths = j.find("paths");
if (paths != j.end())
{
- int interfaceCount = thisSession.interfaces.size();
+ size_t interfaceCount = thisSession.interfaces.size();
if (interfaceCount == 0)
{
interfaceCount = 1;
@@ -160,7 +160,7 @@
// PropertiesChanged
thisSession.matches.reserve(thisSession.matches.size() +
paths->size() *
- (1 + interfaceCount));
+ (1U + interfaceCount));
}
std::string object_manager_match_string;
std::string properties_match_string;
diff --git a/include/dbus_utility.hpp b/include/dbus_utility.hpp
index 2dd3d97..ac5970d 100644
--- a/include/dbus_utility.hpp
+++ b/include/dbus_utility.hpp
@@ -53,9 +53,9 @@
std::string& result)
{
int count = 0;
- auto first = path.begin();
- auto last = path.end();
- for (auto it = path.begin(); it < path.end(); it++)
+ std::string::const_iterator first = path.begin();
+ std::string::const_iterator last = path.end();
+ for (std::string::const_iterator it = path.begin(); it < path.end(); it++)
{
// skip first character as it's either a leading slash or the first
// character in the word
@@ -85,7 +85,8 @@
{
first++;
}
- result = path.substr(first - path.begin(), last - first);
+ result = path.substr(static_cast<size_t>(first - path.begin()),
+ static_cast<size_t>(last - first));
return true;
}
diff --git a/include/image_upload.hpp b/include/image_upload.hpp
index 867d1bc..529e056 100644
--- a/include/image_upload.hpp
+++ b/include/image_upload.hpp
@@ -29,10 +29,10 @@
return;
}
// Make this const static so it survives outside this method
- static boost::asio::deadline_timer timeout(*req.ioService,
- boost::posix_time::seconds(5));
+ static boost::asio::steady_timer timeout(*req.ioService,
+ std::chrono::seconds(5));
- timeout.expires_from_now(boost::posix_time::seconds(15));
+ timeout.expires_after(std::chrono::seconds(15));
auto timeoutHandler = [&res](const boost::system::error_code& ec) {
fwUpdateMatcher = nullptr;
@@ -76,12 +76,7 @@
"xyz.openbmc_project.Software.Version";
}) != interfaces.end())
{
- boost::system::error_code ec;
- timeout.cancel(ec);
- if (ec)
- {
- BMCWEB_LOG_ERROR << "error canceling timer " << ec;
- }
+ timeout.cancel();
std::size_t index = path.str.rfind('/');
if (index != std::string::npos)
diff --git a/include/kvm_websocket.hpp b/include/kvm_websocket.hpp
index db42ab8..9fc3926 100644
--- a/include/kvm_websocket.hpp
+++ b/include/kvm_websocket.hpp
@@ -17,7 +17,7 @@
{
public:
explicit KvmSession(crow::websocket::Connection& conn) :
- conn(conn), doingWrite(false), hostSocket(conn.get_io_context())
+ conn(conn), hostSocket(conn.get_io_context()), doingWrite(false)
{
boost::asio::ip::tcp::endpoint endpoint(
boost::asio::ip::make_address("::1"), 5900);
diff --git a/include/obmc_console.hpp b/include/obmc_console.hpp
index ca723d3..25f3b39 100644
--- a/include/obmc_console.hpp
+++ b/include/obmc_console.hpp
@@ -44,7 +44,7 @@
if (ec == boost::asio::error::eof)
{
- for (auto session : sessions)
+ for (crow::websocket::Connection* session : sessions)
{
session->close("Error in reading to host port");
}
@@ -70,14 +70,14 @@
{
BMCWEB_LOG_ERROR << "Couldn't read from host serial port: "
<< ec;
- for (auto session : sessions)
+ for (crow::websocket::Connection* session : sessions)
{
session->close("Error in connecting to host port");
}
return;
}
std::string_view payload(outputBuffer.data(), bytesRead);
- for (auto session : sessions)
+ for (crow::websocket::Connection* session : sessions)
{
session->sendBinary(payload);
}
@@ -90,7 +90,7 @@
if (ec)
{
BMCWEB_LOG_ERROR << "Couldn't connect to host serial port: " << ec;
- for (auto session : sessions)
+ for (crow::websocket::Connection* session : sessions)
{
session->close("Error in connecting to host port");
}
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index 7839e65..775e6e1 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -564,8 +564,8 @@
{
return -1;
}
- r = sd_bus_message_append_basic(m, argCode[0],
- (void *)stringValue->c_str());
+ r = sd_bus_message_append_basic(
+ m, argCode[0], static_cast<const void *>(stringValue->data()));
if (r < 0)
{
return r;
@@ -1640,8 +1640,8 @@
},
"xyz.openbmc_project.ObjectMapper",
"/xyz/openbmc_project/object_mapper",
- "xyz.openbmc_project.ObjectMapper", "GetSubTree", objectPath,
- static_cast<int32_t>(0), std::array<const char *, 0>());
+ "xyz.openbmc_project.ObjectMapper", "GetSubTree", objectPath, 0,
+ std::array<const char *, 0>());
}
void handleGet(crow::Response &res, std::string &objectPath,
@@ -2500,14 +2500,15 @@
propertiesObj[name];
crow::connections::systemBus->async_send(
m, [&propertyItem, asyncResp](
- boost::system::error_code &ec,
- sdbusplus::message::message &m) {
- if (ec)
+ boost::system::error_code &e,
+ sdbusplus::message::message &msg) {
+ if (e)
{
return;
}
- convertDBusToJSON("v", m, propertyItem);
+ convertDBusToJSON("v", msg,
+ propertyItem);
});
}
property = property->NextSiblingElement("property");
diff --git a/include/pam_authenticate.hpp b/include/pam_authenticate.hpp
index f211a29..1469aef 100644
--- a/include/pam_authenticate.hpp
+++ b/include/pam_authenticate.hpp
@@ -25,7 +25,7 @@
std::strcpy(pass, appPass);
*resp = reinterpret_cast<pam_response*>(
- calloc(numMsg, sizeof(struct pam_response)));
+ calloc(static_cast<size_t>(numMsg), sizeof(struct pam_response)));
if (resp == nullptr)
{
diff --git a/include/persistent_data_middleware.hpp b/include/persistent_data_middleware.hpp
index 1162fc5..5d7e97c 100644
--- a/include/persistent_data_middleware.hpp
+++ b/include/persistent_data_middleware.hpp
@@ -25,7 +25,7 @@
class Middleware
{
- int jsonRevision = 1;
+ uint64_t jsonRevision = 1;
public:
// todo(ed) should read this from a fixed location somewhere, not CWD
@@ -62,7 +62,7 @@
void readData()
{
std::ifstream persistentFile(filename);
- int fileRevision = 0;
+ uint64_t fileRevision = 0;
if (persistentFile.is_open())
{
// call with exceptions disabled
diff --git a/include/sessions.hpp b/include/sessions.hpp
index 2900cd5..b183a0e 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -362,22 +362,22 @@
// https://www.owasp.org/index.php/Session_Management_Cheat_Sheet#Session_ID_Entropy
std::string sessionToken;
sessionToken.resize(20, '0');
- std::uniform_int_distribution<int> dist(0, alphanum.size() - 1);
- for (int i = 0; i < sessionToken.size(); ++i)
+ std::uniform_int_distribution<size_t> dist(0, alphanum.size() - 1);
+ for (size_t i = 0; i < sessionToken.size(); ++i)
{
sessionToken[i] = alphanum[dist(rd)];
}
// Only need csrf tokens for cookie based auth, token doesn't matter
std::string csrfToken;
csrfToken.resize(20, '0');
- for (int i = 0; i < csrfToken.size(); ++i)
+ for (size_t i = 0; i < csrfToken.size(); ++i)
{
csrfToken[i] = alphanum[dist(rd)];
}
std::string uniqueId;
uniqueId.resize(10, '0');
- for (int i = 0; i < uniqueId.size(); ++i)
+ for (size_t i = 0; i < uniqueId.size(); ++i)
{
uniqueId[i] = alphanum[dist(rd)];
}
@@ -449,7 +449,7 @@
{
return needWrite;
}
- int getTimeoutInSeconds() const
+ int64_t getTimeoutInSeconds() const
{
return std::chrono::seconds(timeoutInMinutes).count();
};
diff --git a/include/ssl_key_handler.hpp b/include/ssl_key_handler.hpp
index ce6d9fa..d634d63 100644
--- a/include/ssl_key_handler.hpp
+++ b/include/ssl_key_handler.hpp
@@ -17,10 +17,7 @@
namespace ensuressl
{
static void initOpenssl();
-static void cleanupOpenssl();
-static EVP_PKEY *createRsaKey();
static EVP_PKEY *createEcKey();
-static void handleOpensslError();
// Trust chain related errors.`
inline bool isTrustChainError(int errnum)
@@ -112,7 +109,6 @@
if (file != NULL)
{
EVP_PKEY *pkey = PEM_read_PrivateKey(file, NULL, NULL, NULL);
- int rc;
if (pkey != nullptr)
{
RSA *rsa = EVP_PKEY_get1_RSA(pkey);
@@ -200,7 +196,7 @@
// number If this is not random, regenerating certs throws broswer
// errors
std::random_device rd;
- int serial = rd();
+ int serial = static_cast<int>(rd());
ASN1_INTEGER_set(X509_get_serialNumber(x509), serial);
@@ -254,45 +250,6 @@
// cleanup_openssl();
}
-EVP_PKEY *createRsaKey()
-{
- RSA *pRSA = NULL;
-#if OPENSSL_VERSION_NUMBER < 0x00908000L
- pRSA = RSA_generate_key(2048, RSA_3, NULL, NULL);
-#else
- RSA_generate_key_ex(pRSA, 2048, NULL, NULL);
-#endif
-
- EVP_PKEY *pKey = EVP_PKEY_new();
- if ((pRSA != nullptr) && (pKey != nullptr) &&
- EVP_PKEY_assign_RSA(pKey, pRSA))
- {
- /* pKey owns pRSA from now */
- if (RSA_check_key(pRSA) <= 0)
- {
- fprintf(stderr, "RSA_check_key failed.\n");
- handleOpensslError();
- EVP_PKEY_free(pKey);
- pKey = NULL;
- }
- }
- else
- {
- handleOpensslError();
- if (pRSA != nullptr)
- {
- RSA_free(pRSA);
- pRSA = NULL;
- }
- if (pKey != nullptr)
- {
- EVP_PKEY_free(pKey);
- pKey = NULL;
- }
- }
- return pKey;
-}
-
EVP_PKEY *createEcKey()
{
EVP_PKEY *pKey = NULL;
@@ -329,20 +286,6 @@
#endif
}
-void cleanupOpenssl()
-{
- CRYPTO_cleanup_all_ex_data();
- ERR_free_strings();
-#if OPENSSL_VERSION_NUMBER < 0x10100000L
- ERR_remove_thread_state(0);
-#endif
- EVP_cleanup();
-}
-
-void handleOpensslError()
-{
- ERR_print_errors_fp(stderr);
-}
inline void ensureOpensslKeyPresentAndValid(const std::string &filepath)
{
bool pemFileValid = false;
diff --git a/include/vm_websocket.hpp b/include/vm_websocket.hpp
index 3f229e6..6485920 100644
--- a/include/vm_websocket.hpp
+++ b/include/vm_websocket.hpp
@@ -23,8 +23,8 @@
class Handler : public std::enable_shared_from_this<Handler>
{
public:
- Handler(const std::string& media, boost::asio::io_service& ios) :
- pipeOut(ios), pipeIn(ios), media(media), doingWrite(false),
+ Handler(const std::string& mediaIn, boost::asio::io_context& ios) :
+ pipeOut(ios), pipeIn(ios), media(mediaIn), doingWrite(false),
outputBuffer(new boost::beast::flat_static_buffer<nbdBufferSize>),
inputBuffer(new boost::beast::flat_static_buffer<nbdBufferSize>)
{