Various cleanups
* Move functions to private as appropriate
* Reorder functions to match declaration order
* Declare arguments as const when appropriate
* Rename to getActionHandler() to match previous method name
Signed-off-by: Kun Yi <kunyi731@gmail.com>
Change-Id: I7d3b5bbb07c218c6c2d6277a1f19d8a0299bd6e6
diff --git a/manager.cpp b/manager.cpp
index 64cc08f..894e743 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -25,63 +25,6 @@
namespace blobs
{
-void BlobManager::eraseSession(GenericBlobInterface* handler, uint16_t session)
-{
- if (auto item = sessions.find(session); item != sessions.end())
- {
- const auto& blobId = item->second.blobId;
-
- /* Ok for openSessions[handler] to be an empty set */
- openSessions[handler].erase(session);
- openFiles[blobId]--;
- if (openFiles[blobId] == 0)
- {
- openFiles.erase(blobId);
- }
-
- /* Erase at the end after using the session info */
- sessions.erase(session);
- }
-}
-
-void BlobManager::cleanUpStaleSessions(GenericBlobInterface* handler)
-{
- if (openSessions.count(handler) == 0)
- {
- return;
- }
-
- auto timeNow = std::chrono::steady_clock::now();
- std::set<uint16_t> expiredSet;
-
- for (auto sessionId : openSessions[handler])
- {
- if (timeNow - sessions[sessionId].lastActionTime >= sessionTimeout)
- {
- expiredSet.insert(sessionId);
- }
- }
-
- for (auto sessionId : expiredSet)
- {
- std::cerr << "phosphor-ipmi-blobs: expiring stale session " << sessionId
- << std::endl;
-
- /* We do a best case recovery by issuing an expire call. If it fails
- * don't erase sessions since the handler side might be still tracking
- * it as open. */
- if (handler->expire(sessionId))
- {
- eraseSession(handler, sessionId);
- }
- else
- {
- std::cerr << "phosphor-ipmi-blobs: failed to expire session "
- << sessionId << std::endl;
- }
- }
-}
-
bool BlobManager::registerHandler(std::unique_ptr<GenericBlobInterface> handler)
{
if (!handler)
@@ -161,32 +104,6 @@
return true;
}
-GenericBlobInterface* BlobManager::getHandler(const std::string& path)
-{
- /* Find a handler. */
- auto h = std::find_if(
- handlers.begin(), handlers.end(),
- [&path](const auto& iter) { return (iter->canHandleBlob(path)); });
- if (h != handlers.end())
- {
- return h->get();
- }
-
- return nullptr;
-}
-
-GenericBlobInterface* BlobManager::getActionHandle(uint16_t session,
- uint16_t requiredFlags)
-{
- if (auto item = sessions.find(session);
- item != sessions.end() && (item->second.flags & requiredFlags))
- {
- item->second.lastActionTime = std::chrono::steady_clock::now();
- return item->second.handler;
- }
- return nullptr;
-}
-
bool BlobManager::stat(const std::string& path, BlobMeta* meta)
{
/* meta should never be NULL. */
@@ -203,7 +120,7 @@
bool BlobManager::stat(uint16_t session, BlobMeta* meta)
{
- if (auto handler = getActionHandle(session))
+ if (auto handler = getActionHandler(session))
{
return handler->stat(session, meta);
}
@@ -212,7 +129,7 @@
bool BlobManager::commit(uint16_t session, const std::vector<uint8_t>& data)
{
- if (auto handler = getActionHandle(session))
+ if (auto handler = getActionHandler(session))
{
return handler->commit(session, data);
}
@@ -221,7 +138,7 @@
bool BlobManager::close(uint16_t session)
{
- if (auto handler = getActionHandle(session))
+ if (auto handler = getActionHandler(session))
{
if (!handler->close(session))
{
@@ -250,7 +167,7 @@
*/
// uint32_t maxTransportSize = ipmi::getChannelMaxTransferSize(ipmiChannel);
- if (auto handler = getActionHandle(session, OpenFlags::read))
+ if (auto handler = getActionHandler(session, OpenFlags::read))
{
return handler->read(session, offset,
std::min(maximumReadSize, requestedSize));
@@ -261,7 +178,7 @@
bool BlobManager::write(uint16_t session, uint32_t offset,
const std::vector<uint8_t>& data)
{
- if (auto handler = getActionHandle(session, OpenFlags::write))
+ if (auto handler = getActionHandler(session, OpenFlags::write))
{
return handler->write(session, offset, data);
}
@@ -291,7 +208,7 @@
bool BlobManager::writeMeta(uint16_t session, uint32_t offset,
const std::vector<uint8_t>& data)
{
- if (auto handler = getActionHandle(session))
+ if (auto handler = getActionHandler(session))
{
return handler->writeMeta(session, offset, data);
}
@@ -326,6 +243,90 @@
return false;
}
+GenericBlobInterface* BlobManager::getHandler(const std::string& path)
+{
+ /* Find a handler. */
+ auto h = std::find_if(
+ handlers.begin(), handlers.end(),
+ [&path](const auto& iter) { return (iter->canHandleBlob(path)); });
+ if (h != handlers.end())
+ {
+ return h->get();
+ }
+
+ return nullptr;
+}
+
+GenericBlobInterface* BlobManager::getActionHandler(uint16_t session,
+ uint16_t requiredFlags)
+{
+ if (auto item = sessions.find(session);
+ item != sessions.end() && (item->second.flags & requiredFlags))
+ {
+ item->second.lastActionTime = std::chrono::steady_clock::now();
+ return item->second.handler;
+ }
+ return nullptr;
+}
+
+void BlobManager::eraseSession(GenericBlobInterface* const handler,
+ uint16_t session)
+{
+ if (auto item = sessions.find(session); item != sessions.end())
+ {
+ const auto& blobId = item->second.blobId;
+
+ /* Ok for openSessions[handler] to be an empty set */
+ openSessions[handler].erase(session);
+ openFiles[blobId]--;
+ if (openFiles[blobId] == 0)
+ {
+ openFiles.erase(blobId);
+ }
+
+ /* Erase at the end after using the session info */
+ sessions.erase(session);
+ }
+}
+
+void BlobManager::cleanUpStaleSessions(GenericBlobInterface* const handler)
+{
+ if (openSessions.count(handler) == 0)
+ {
+ return;
+ }
+
+ auto timeNow = std::chrono::steady_clock::now();
+ std::set<uint16_t> expiredSet;
+
+ for (auto sessionId : openSessions[handler])
+ {
+ if (timeNow - sessions[sessionId].lastActionTime >= sessionTimeout)
+ {
+ expiredSet.insert(sessionId);
+ }
+ }
+
+ for (auto sessionId : expiredSet)
+ {
+ std::cerr << "phosphor-ipmi-blobs: expiring stale session " << sessionId
+ << std::endl;
+
+ /* We do a best case recovery by issuing an expire call. If it fails
+ * don't erase sessions since the handler side might be still tracking
+ * it as open. */
+ if (handler->expire(sessionId))
+ {
+ eraseSession(handler, sessionId);
+ }
+ else
+ {
+ std::cerr << "phosphor-ipmi-blobs: failed to expire session "
+ << sessionId << std::endl;
+ }
+ }
+}
+
static std::unique_ptr<BlobManager> manager;
ManagerInterface* getBlobManager()
diff --git a/manager.hpp b/manager.hpp
index a7e9936..3dc09cd 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -236,6 +236,7 @@
*/
bool getSession(uint16_t* session);
+ private:
/**
* Given a file path will return first handler to answer that it owns
* it.
@@ -246,23 +247,34 @@
GenericBlobInterface* getHandler(const std::string& path);
/**
- * Given a session id, update session time and return a handle to take
+ * Given a session id, update session time and return a handler to take
* action
*
* @param[in] session - session ID
- * @param[in] requiredFlags - only return handle if the flags for this
+ * @param[in] requiredFlags - only return handler if the flags for this
* session contain these flags; defaults to any flag
* @return session handler, nullptr if cannot get handler
*/
- GenericBlobInterface* getActionHandle(
+ GenericBlobInterface* getActionHandler(
uint16_t session,
uint16_t requiredFlags = std::numeric_limits<uint16_t>::max());
- private:
- /* Helper method to erase a session from all maps */
- void eraseSession(GenericBlobInterface* handler, uint16_t session);
- /* For each session owned by this handler, call expire if it is stale */
- void cleanUpStaleSessions(GenericBlobInterface* handler);
+ /**
+ * Helper method to erase a session from all maps
+ *
+ * @param[in] handler - handler pointer for lookup
+ * @param[in] session - session ID for lookup
+ * @return None
+ */
+ void eraseSession(GenericBlobInterface* const handler, uint16_t session);
+
+ /**
+ * For each session owned by this handler, call expire if it is stale
+ *
+ * @param[in] handler - handler pointer for lookup
+ * @return None
+ */
+ void cleanUpStaleSessions(GenericBlobInterface* const handler);
/* How long a session has to be inactive to be considered stale */
std::chrono::seconds sessionTimeout;