From 3fba1ebb9ce6503b15605e6c0d1d282109146822 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Fri, 6 Feb 2015 12:49:43 +0100 Subject: [PATCH] Code cleanup [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, run tests Change-Id: I8911fcc2e51bd14fa94eac6d75e655ba1ce275d8 --- common/ipc/internals/processor.hpp | 2 +- common/ipc/internals/socket.cpp | 4 +- common/ipc/internals/socket.hpp | 9 ++- common/lxc/zone.cpp | 4 +- common/utils/fs.cpp | 20 +++--- server/zone-provision.cpp | 4 +- server/zones-manager.cpp | 123 +++++++++++++++++-------------------- server/zones-manager.hpp | 2 + 8 files changed, 83 insertions(+), 85 deletions(-) diff --git a/common/ipc/internals/processor.hpp b/common/ipc/internals/processor.hpp index 604d863..b890cda 100644 --- a/common/ipc/internals/processor.hpp +++ b/common/ipc/internals/processor.hpp @@ -219,7 +219,7 @@ public: * Synchronous method call. * * @param methodID API dependent id of the method - * @param peerD id of the peer + * @param peerID id of the peer * @param data data to sent * @param timeoutMS how long to wait for the return value before throw * @tparam SentDataType data type to send diff --git a/common/ipc/internals/socket.cpp b/common/ipc/internals/socket.cpp index b12dfa0..2469af6 100644 --- a/common/ipc/internals/socket.cpp +++ b/common/ipc/internals/socket.cpp @@ -51,13 +51,13 @@ Socket::Socket(int socketFD) { } -Socket::Socket(Socket&& socket) +Socket::Socket(Socket&& socket) noexcept : mFD(socket.mFD) { socket.mFD = -1; } -Socket::~Socket() +Socket::~Socket() noexcept { try { ipc::close(mFD); diff --git a/common/ipc/internals/socket.hpp b/common/ipc/internals/socket.hpp index 91f0dcd..839afe4 100644 --- a/common/ipc/internals/socket.hpp +++ b/common/ipc/internals/socket.hpp @@ -48,10 +48,13 @@ public: * * @param socketFD socket obtained outside the class. */ - Socket(int socketFD = -1); - Socket(Socket&& socket); - ~Socket(); + explicit Socket(int socketFD = -1); + Socket(Socket&& socket) noexcept; + ~Socket() noexcept; + + Socket(const Socket&) = delete; Socket& operator=(const Socket&) = delete; + Socket& operator=(Socket&&) = delete; /** * @return reference to the socket's file descriptor diff --git a/common/lxc/zone.cpp b/common/lxc/zone.cpp index e4371f8..dc148e4 100644 --- a/common/lxc/zone.cpp +++ b/common/lxc/zone.cpp @@ -316,8 +316,8 @@ bool LxcZone::waitForState(State state, int timeout) bool LxcZone::setRunLevel(int runLevel) { auto callback = [](void* param) -> int { - utils::RunLevel runLevel = *reinterpret_cast(param); - return utils::setRunLevel(runLevel) ? 0 : 1; + utils::RunLevel level = *reinterpret_cast(param); + return utils::setRunLevel(level) ? 0 : 1; }; lxc_attach_options_t options = LXC_ATTACH_OPTIONS_DEFAULT; diff --git a/common/utils/fs.cpp b/common/utils/fs.cpp index 763bdfe..b2260cc 100644 --- a/common/utils/fs.cpp +++ b/common/utils/fs.cpp @@ -363,23 +363,25 @@ bool createDir(const std::string& path, uid_t uid, uid_t gid, boost::filesystem: bool createDirs(const std::string& path, mode_t mode) { - boost::filesystem::perms perms = getPerms(mode); - std::vector dirs; + const boost::filesystem::perms perms = getPerms(mode); + std::vector dirsCreated; fs::path prefix; - fs::path dirPath = fs::path(path); - for (const auto dir : dirPath) { - prefix /= dir; + const fs::path dirPath = fs::path(path); + for (const auto& dirSegment : dirPath) { + prefix /= dirSegment; if (!fs::exists(prefix)) { bool created = createDir(prefix.string(), -1, -1, perms); if (created) { - dirs.push_back(prefix); + dirsCreated.push_back(prefix); } else { LOGE("Failed to create dir"); - for (auto dir = dirs.rbegin(); dir != dirs.rend(); ++dir) { + // undo + for (auto iter = dirsCreated.rbegin(); iter != dirsCreated.rend(); ++iter) { boost::system::error_code errorCode; - fs::remove(*dir, errorCode); + fs::remove(*iter, errorCode); if (errorCode) { - LOGE("Error during cleaning: dir: " << *dir << ", msg: " << errorCode.message()); + LOGE("Error during cleaning: dir: " << *iter + << ", msg: " << errorCode.message()); } } return false; diff --git a/server/zone-provision.cpp b/server/zone-provision.cpp index 861a506..1c542f7 100644 --- a/server/zone-provision.cpp +++ b/server/zone-provision.cpp @@ -71,8 +71,8 @@ std::string ZoneProvision::declareProvision(ZoneProvisioningConfig::Provision&& std::string id = getId(provision); auto it = std::find_if(mProvisioningConfig.provisions.begin(), mProvisioningConfig.provisions.end(), - [&](const ZoneProvisioningConfig::Provision& provision) { - return getId(provision) == id; + [&](const ZoneProvisioningConfig::Provision& existingProvision) { + return getId(existingProvision) == id; }); if (it != mProvisioningConfig.provisions.end()) { LOGE("Can't add provision. It already exists: " << id); diff --git a/server/zones-manager.cpp b/server/zones-manager.cpp index 34e1531..167f9f4 100644 --- a/server/zones-manager.cpp +++ b/server/zones-manager.cpp @@ -106,29 +106,11 @@ Iter circularFindNext(Iter begin, Iter end, Iter current, Predicate pred) } } -std::vector>::iterator find(std::vector>& zones, - const std::string& id) -{ - auto equalId = [&id](const std::unique_ptr& zone) { - return zone->getId() == id; - }; - return std::find_if(zones.begin(), zones.end(), equalId); -} - Zone& get(std::vector>::iterator iter) { return **iter; } -Zone& at(std::vector>& zones, const std::string& id) -{ - auto iter = find(zones, id); - if (iter == zones.end()) { - throw std::out_of_range("id not found"); - } - return get(iter); -} - bool zoneIsRunning(const std::unique_ptr& zone) { return zone->isRunning(); } @@ -240,6 +222,22 @@ ZonesManager::~ZonesManager() LOGD("ZonesManager object destroyed"); } +ZonesManager::Zones::iterator ZonesManager::findZone(const std::string& id) +{ + return std::find_if(mZones.begin(), mZones.end(), [&id](const std::unique_ptr& zone) { + return zone->getId() == id; + }); +} + +Zone& ZonesManager::getZone(const std::string& id) +{ + auto iter = findZone(id); + if (iter == mZones.end()) { + throw std::out_of_range("id not found"); + } + return get(iter); +} + void ZonesManager::saveDynamicConfig() { config::saveToKVStore(mConfig.dbPath, mDynamicConfig, DB_PREFIX); @@ -252,7 +250,7 @@ void ZonesManager::updateDefaultId() LOGT("Keep empty defaultId"); return; } - if (find(mZones, mDynamicConfig.defaultId) != mZones.end()) { + if (findZone(mDynamicConfig.defaultId) != mZones.end()) { LOGT("Keep " << mDynamicConfig.defaultId << " as defaultId"); return; } @@ -300,7 +298,7 @@ void ZonesManager::createZone(const std::string& zoneConfigPath) Lock lock(mMutex); - if (find(mZones, id) != mZones.end()) { + if (findZone(id) != mZones.end()) { throw ZoneOperationException("Zone already exists"); } mZones.push_back(std::move(zone)); @@ -318,7 +316,7 @@ void ZonesManager::destroyZone(const std::string& zoneId) { Lock lock(mMutex); - auto iter = find(mZones, zoneId); + auto iter = findZone(zoneId); if (iter == mZones.end()) { LOGE("Failed to destroy zone " << zoneId << ": no such zone"); throw ZoneOperationException("No such zone"); @@ -344,7 +342,7 @@ void ZonesManager::destroyZone(const std::string& zoneId) void ZonesManager::focus(const std::string& zoneId) { Lock lock(mMutex); - auto iter = find(mZones, zoneId); + auto iter = findZone(zoneId); focusInternal(iter); } @@ -361,29 +359,29 @@ void ZonesManager::focusInternal(Zones::iterator iter) return; } - Zone& zone = get(iter); - std::string zoneId = zone.getId(); + Zone& zoneToFocus = get(iter); + const std::string idToFocus = zoneToFocus.getId(); - if (zoneId == mActiveZoneId) { + if (idToFocus == mActiveZoneId) { return; } - if (!zone.isRunning()) { - LOGE("Can't focus not running zone " << zoneId); + if (!zoneToFocus.isRunning()) { + LOGE("Can't focus not running zone " << idToFocus); assert(false); return; } - LOGI("Focus to: " << zone.getId()); + LOGI("Focus to: " << idToFocus); - if (!zone.activateVT()) { + if (!zoneToFocus.activateVT()) { LOGE("Failed to activate zones VT"); return; } for (auto& zone : mZones) { std::string id = zone->getId(); - if (id == zoneId) { + if (id == idToFocus) { LOGD(id << ": being sent to foreground"); zone->goForeground(); } else { @@ -391,7 +389,7 @@ void ZonesManager::focusInternal(Zones::iterator iter) zone->goBackground(); } } - mActiveZoneId = zoneId; + mActiveZoneId = idToFocus; } void ZonesManager::refocus() @@ -399,25 +397,18 @@ void ZonesManager::refocus() // assume mutex is locked // check if refocus is required - auto oldIter = find(mZones, mActiveZoneId); + auto oldIter = findZone(mActiveZoneId); if (oldIter != mZones.end() && get(oldIter).isRunning()) { return; } // try to refocus to defaultId - auto iter = find(mZones, mDynamicConfig.defaultId); - if (iter != mZones.end() && get(iter).isRunning()) { - // focus to default - focusInternal(iter); - } else { + auto iter = findZone(mDynamicConfig.defaultId); + if (iter == mZones.end() || !get(iter).isRunning()) { // focus to any running or to host if not found - auto iter = std::find_if(mZones.begin(), mZones.end(), zoneIsRunning); - if (iter == mZones.end()) { - focusInternal(mZones.end()); - } else { - focusInternal(iter); - } + iter = std::find_if(mZones.begin(), mZones.end(), zoneIsRunning); } + focusInternal(iter); } void ZonesManager::startAll() @@ -450,7 +441,7 @@ bool ZonesManager::isPaused(const std::string& zoneId) { Lock lock(mMutex); - auto iter = find(mZones, zoneId); + auto iter = findZone(zoneId); if (iter == mZones.end()) { LOGE("No such zone id: " << zoneId); throw ZoneOperationException("No such zone"); @@ -463,7 +454,7 @@ bool ZonesManager::isRunning(const std::string& zoneId) { Lock lock(mMutex); - auto iter = find(mZones, zoneId); + auto iter = findZone(zoneId); if (iter == mZones.end()) { LOGE("No such zone id: " << zoneId); throw ZoneOperationException("No such zone"); @@ -491,7 +482,7 @@ ZonesManager::Zones::iterator ZonesManager::getRunningForegroundZoneIterator() if (mActiveZoneId.empty()) { return mZones.end(); } - auto iter = find(mZones, mActiveZoneId); + auto iter = findZone(mActiveZoneId); if (!get(iter).isRunning()) { // Can zone change its state by itself? // Maybe when it is shut down by itself? TODO check it @@ -505,7 +496,7 @@ ZonesManager::Zones::iterator ZonesManager::getRunningForegroundZoneIterator() ZonesManager::Zones::iterator ZonesManager::getNextToForegroundZoneIterator() { // assume mutex is locked - auto current = find(mZones, mActiveZoneId); + auto current = findZone(mActiveZoneId); if (current == mZones.end()) { // find any running return std::find_if(mZones.begin(), mZones.end(), zoneIsRunning); @@ -564,8 +555,8 @@ void ZonesManager::displayOffHandler(const std::string& /*caller*/) // get config of currently set zone and switch if switchToDefaultAfterTimeout is true Lock lock(mMutex); - auto activeIter = find(mZones, mActiveZoneId); - auto defaultIter = find(mZones, mDynamicConfig.defaultId); + auto activeIter = findZone(mActiveZoneId); + auto defaultIter = findZone(mDynamicConfig.defaultId); if (activeIter != mZones.end() && defaultIter != mZones.end() && @@ -608,14 +599,14 @@ void ZonesManager::handleZoneMoveFileRequest(const std::string& srcZoneId, Lock lock(mMutex); - auto srcIter = find(mZones, srcZoneId); + auto srcIter = findZone(srcZoneId); if (srcIter == mZones.end()) { LOGE("Source zone '" << srcZoneId << "' not found"); return; } Zone& srcZone = get(srcIter); - auto dstIter = find(mZones, dstZoneId); + auto dstIter = findZone(dstZoneId); if (dstIter == mZones.end()) { LOGE("Destination zone '" << dstZoneId << "' not found"); result->set(g_variant_new("(s)", api::zone::FILE_MOVE_DESTINATION_NOT_FOUND.c_str())); @@ -703,7 +694,7 @@ void ZonesManager::handleProxyCall(const std::string& caller, Lock lock(mMutex); - auto targetIter = find(mZones, target); + auto targetIter = findZone(target); if (targetIter == mZones.end()) { LOGE("Target zone '" << target << "' not found"); result->setError(api::ERROR_INVALID_ID, "Unknown proxy call target"); @@ -770,7 +761,7 @@ void ZonesManager::handleGetZoneInfoCall(const std::string& id, Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("No zone with id=" << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -810,7 +801,7 @@ void ZonesManager::handleDeclareFileCall(const std::string& zone, try { Lock lock(mMutex); - const std::string id = at(mZones, zone).declareFile(type, path, flags, mode); + const std::string id = getZone(zone).declareFile(type, path, flags, mode); result->set(g_variant_new("(s)", id.c_str())); } catch (const std::out_of_range&) { LOGE("No zone with id=" << zone); @@ -834,7 +825,7 @@ void ZonesManager::handleDeclareMountCall(const std::string& source, try { Lock lock(mMutex); - const std::string id = at(mZones, zone).declareMount(source, target, type, flags, data); + const std::string id = getZone(zone).declareMount(source, target, type, flags, data); result->set(g_variant_new("(s)", id.c_str())); } catch (const std::out_of_range&) { LOGE("No zone with id=" << zone); @@ -854,7 +845,7 @@ void ZonesManager::handleDeclareLinkCall(const std::string& source, try { Lock lock(mMutex); - const std::string id = at(mZones, zone).declareLink(source, target); + const std::string id = getZone(zone).declareLink(source, target); result->set(g_variant_new("(s)", id.c_str())); } catch (const std::out_of_range&) { LOGE("No zone with id=" << zone); @@ -872,7 +863,7 @@ void ZonesManager::handleGetDeclarationsCall(const std::string& zone, try { Lock lock(mMutex); - std::vector declarations = at(mZones, zone).getDeclarations(); + std::vector declarations = getZone(zone).getDeclarations(); std::vector out; for (auto declaration : declarations) { @@ -901,7 +892,7 @@ void ZonesManager::handleRemoveDeclarationCall(const std::string& zone, try { Lock lock(mMutex); - at(mZones, zone).removeDeclaration(declarationId); + getZone(zone).removeDeclaration(declarationId); result->setVoid(); } catch (const std::out_of_range&) { @@ -920,7 +911,7 @@ void ZonesManager::handleSetActiveZoneCall(const std::string& id, Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()){ LOGE("No zone with id=" << id ); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -1024,7 +1015,7 @@ void ZonesManager::handleCreateZoneCall(const std::string& id, namespace fs = boost::filesystem; // check if zone does not exist - if (find(mZones, id) != mZones.end()) { + if (findZone(id) != mZones.end()) { LOGE("Cannot create " << id << " zone - already exists!"); result->setError(api::ERROR_INVALID_ID, "Already exists"); return; @@ -1120,7 +1111,7 @@ void ZonesManager::handleShutdownZoneCall(const std::string& id, LOGT("Shutdown zone " << id); Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("Failed to shutdown zone - no such zone id: " << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -1149,7 +1140,7 @@ void ZonesManager::handleStartZoneCall(const std::string& id, LOGT("Start zone " << id ); Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("Failed to start zone - no such zone id: " << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -1173,7 +1164,7 @@ void ZonesManager::handleLockZoneCall(const std::string& id, Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("Failed to lock zone - no such zone id: " << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -1207,7 +1198,7 @@ void ZonesManager::handleUnlockZoneCall(const std::string& id, Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("Failed to unlock zone - no such zone id: " << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -1242,7 +1233,7 @@ void ZonesManager::handleGrantDeviceCall(const std::string& id, Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("Failed to grant device - no such zone id: " << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); @@ -1282,7 +1273,7 @@ void ZonesManager::handleRevokeDeviceCall(const std::string& id, Lock lock(mMutex); - auto iter = find(mZones, id); + auto iter = findZone(id); if (iter == mZones.end()) { LOGE("Failed to revoke device - no such zone id: " << id); result->setError(api::ERROR_INVALID_ID, "No such zone id"); diff --git a/server/zones-manager.hpp b/server/zones-manager.hpp index 9ede4e9..755bc97 100644 --- a/server/zones-manager.hpp +++ b/server/zones-manager.hpp @@ -123,6 +123,8 @@ private: std::string mActiveZoneId; bool mDetachOnExit; + Zones::iterator findZone(const std::string& id); + Zone& getZone(const std::string& id); Zones::iterator getRunningForegroundZoneIterator(); Zones::iterator getNextToForegroundZoneIterator(); void focusInternal(Zones::iterator iter); -- 2.7.4