Skip to content

Commit

Permalink
Refactor: Fix issues found by cppcheck
Browse files Browse the repository at this point in the history
Resolve several issues found by cppcheck:

    [msl_verify.hpp:28]: (style) Class 'MinimumShipLevel' has a constructor with 1 argument that is not explicit.
    [ubi/watch.hpp:21]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it?
    [item_updater_main.cpp:22] -> [item_updater_main.cpp:49]: (style) Local variable rc shadows outer symbol
    [serialize.cpp:19]: (performance) Function parameter 'versionId' should be passed by const reference.
    [ubi/watch.hpp:43]: (style) Struct 'CustomFd' has a constructor with 1 argument that is not explicit.
    [serialize.cpp:52]: (performance) Function parameter 'versionId' should be passed by const reference.
    [serialize.cpp:116]: (performance) Function parameter 'versionId' should be passed by const reference.
    [activation.cpp:115] -> [activation.cpp:152]: (style) Local variable mapperResponseMsg shadows outer symbol
    [version.hpp:114]: (performance) Variable 'eraseCallback' is assigned in constructor body. Consider performing initialization in initialization list.
    [image_verify.hpp:53]: (style) Struct 'CustomFd' has a constructor with 1 argument that is not explicit.
    [ubi/item_updater_ubi.cpp:192]: (performance) Function parameter 'versionId' should be passed by const reference.
    [ubi/item_updater_ubi.cpp:203]: (performance) Function parameter 'versionId' should be passed by const reference.

Tested: Verify the code compiles and cppcheck does not report the above
        issues.

Change-Id: I096392a2a7a283fe198c9c29185125e61295e10f
Signed-off-by: Lei YU <[email protected]>
  • Loading branch information
mine260309 committed Mar 13, 2019
1 parent 0783076 commit 1db9adf
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 20 deletions.
2 changes: 1 addition & 1 deletion activation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ void Activation::deleteImageManagerObject()
deleteInterface, "Delete");
try
{
auto mapperResponseMsg = bus.call(method);
mapperResponseMsg = bus.call(method);

// Check that the bus call didn't result in an error
if (mapperResponseMsg.is_method_error())
Expand Down
2 changes: 1 addition & 1 deletion image_verify.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ struct CustomFd
*
* @param[in] fd - File descriptor
*/
CustomFd(int fd) : fd(fd)
explicit CustomFd(int fd) : fd(fd)
{
}

Expand Down
2 changes: 1 addition & 1 deletion item_updater_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ int main(int argc, char* argv[])
&updater, std::placeholders::_1));
#endif
bus.attach_event(loop, SD_EVENT_PRIORITY_NORMAL);
auto rc = sd_event_loop(loop);
rc = sd_event_loop(loop);
if (rc < 0)
{
log<level::ERR>("Error occurred during the sd_event_loop",
Expand Down
2 changes: 1 addition & 1 deletion msl_verify.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class MinimumShipLevel
/** @brief Constructs MinimumShipLevel.
* @param[in] minShipLevel - Minimum Ship Level string
*/
MinimumShipLevel(const std::string& minShipLevel) :
explicit MinimumShipLevel(const std::string& minShipLevel) :
minShipLevel(minShipLevel){};

/** @brief Verify if the current PNOR version meets the min ship level
Expand Down
4 changes: 2 additions & 2 deletions ubi/item_updater_ubi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ int ItemUpdaterUbi::validateSquashFSImage(const std::string& filePath)
}
}

void ItemUpdaterUbi::removeReadOnlyPartition(std::string versionId)
void ItemUpdaterUbi::removeReadOnlyPartition(const std::string& versionId)
{
auto serviceFile = "obmc-flash-bios-ubiumount-ro@" + versionId + ".service";

Expand All @@ -200,7 +200,7 @@ void ItemUpdaterUbi::removeReadOnlyPartition(std::string versionId)
bus.call_noreply(method);
}

void ItemUpdaterUbi::removeReadWritePartition(std::string versionId)
void ItemUpdaterUbi::removeReadWritePartition(const std::string& versionId)
{
auto serviceFile = "obmc-flash-bios-ubiumount-rw@" + versionId + ".service";

Expand Down
4 changes: 2 additions & 2 deletions ubi/item_updater_ubi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,14 @@ class ItemUpdaterUbi : public ItemUpdater
*
* @param[in] versionId - The id of the ro partition to remove.
*/
void removeReadOnlyPartition(std::string versionId);
void removeReadOnlyPartition(const std::string& versionId);

/** @brief Clears read write PNOR partition for
* given Activation D-Bus object
*
* @param[in] versionId - The id of the rw partition to remove.
*/
void removeReadWritePartition(std::string versionId);
void removeReadWritePartition(const std::string& versionId);

/** @brief Clears preserved PNOR partition */
void removePreservedPartition();
Expand Down
6 changes: 3 additions & 3 deletions ubi/serialize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace updater

namespace fs = std::experimental::filesystem;

void storeToFile(std::string versionId, uint8_t priority)
void storeToFile(const std::string& versionId, uint8_t priority)
{
auto bus = sdbusplus::bus::new_default();

Expand Down Expand Up @@ -49,7 +49,7 @@ void storeToFile(std::string versionId, uint8_t priority)
bus.call_noreply(method);
}

bool restoreFromFile(std::string versionId, uint8_t& priority)
bool restoreFromFile(const std::string& versionId, uint8_t& priority)
{
auto varPath = PERSIST_DIR + versionId;
if (fs::exists(varPath))
Expand Down Expand Up @@ -113,7 +113,7 @@ bool restoreFromFile(std::string versionId, uint8_t& priority)
return false;
}

void removeFile(std::string versionId)
void removeFile(const std::string& versionId)
{
auto bus = sdbusplus::bus::new_default();

Expand Down
6 changes: 3 additions & 3 deletions ubi/serialize.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ namespace updater
* @param[in] versionId - The version for which to store information.
* @param[in] priority - RedundancyPriority value for that version.
*/
void storeToFile(std::string versionId, uint8_t priority);
void storeToFile(const std::string& versionId, uint8_t priority);

/** @brief Serialization function - restores activation information from file
* @param[in] versionId - The version for which to retrieve information.
* @param[in] priority - RedundancyPriority pointer for that version.
* @return true if restore was successful, false if not
*/
bool restoreFromFile(std::string versionId, uint8_t& priority);
bool restoreFromFile(const std::string& versionId, uint8_t& priority);

/** @brief Removes the serial file for a given version.
* @param[in] versionId - The version for which to remove a file, if it exists.
*/
void removeFile(std::string versionId);
void removeFile(const std::string& versionId);

} // namespace updater
} // namespace software
Expand Down
4 changes: 2 additions & 2 deletions ubi/watch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ struct EventSourceDeleter
{
void operator()(sd_event_source* eventSource) const
{
eventSource = sd_event_source_unref(eventSource);
sd_event_source_unref(eventSource);
}
};
using EventSourcePtr = std::unique_ptr<sd_event_source, EventSourceDeleter>;
Expand All @@ -40,7 +40,7 @@ struct CustomFd
*
* @param[in] fd - File descriptor
*/
CustomFd(int fd) : fd(fd)
explicit CustomFd(int fd) : fd(fd)
{
}

Expand Down
6 changes: 2 additions & 4 deletions version.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ class Version : public VersionInherit
const std::string& versionString, VersionPurpose versionPurpose,
const std::string& filePath, eraseFunc callback) :
VersionInherit(bus, (objPath).c_str(), true),
bus(bus), objPath(objPath), parent(parent), versionId(versionId),
versionStr(versionString),
eraseCallback(callback), bus(bus), objPath(objPath), parent(parent),
versionId(versionId), versionStr(versionString),
chassisStateSignals(
bus,
sdbusRule::type::signal() + sdbusRule::member("PropertiesChanged") +
Expand All @@ -110,8 +110,6 @@ class Version : public VersionInherit
std::bind(std::mem_fn(&Version::updateDeleteInterface), this,
std::placeholders::_1))
{
// Bind erase method
eraseCallback = callback;
// Set properties.
purpose(versionPurpose);
version(versionString);
Expand Down

0 comments on commit 1db9adf

Please sign in to comment.