-
Notifications
You must be signed in to change notification settings - Fork 14
Description
I noticed that the function LaunchPointList::getByLaunchPointId can return a nullptr that is not always checked.
sam/src/base/LaunchPointList.cpp
Lines 115 to 126 in af986e6
| LaunchPointPtr LaunchPointList::getByLaunchPointId(const string& launchPointId) | |
| { | |
| if (launchPointId.empty()) | |
| return nullptr; | |
| for (auto it = m_list.begin(); it != m_list.end(); ++it) { | |
| if ((*it)->getLaunchPointId() == launchPointId) { | |
| return *it; | |
| } | |
| } | |
| return nullptr; | |
| } |
Consider the following lines:
sam/src/manager/PolicyManager.cpp
Lines 164 to 169 in af986e6
| void PolicyManager::onCloseForRemove(LunaTaskPtr lunaTask) | |
| { | |
| lunaTask->setSuccessCallback(boost::bind(&PolicyManager::onReplyWithoutIds, this, boost::placeholders::_1)); | |
| LaunchPointPtr launchPoint = LaunchPointList::getInstance().getByLaunchPointId(lunaTask->getLaunchPointId()); | |
| switch(launchPoint->getType()) { |
The variable launchPoint could be null, but getType() member function is called without any check.
I found the same issue in DB8::onFind function.
Lines 212 to 220 in af986e6
| launchPoint = LaunchPointList::getInstance().getByLaunchPointId(launchPointId); | |
| if (type == "default") { | |
| launchPoint->setDatabase(results[i]); | |
| } else if (type == "bookmark") { | |
| if (launchPoint == nullptr) { | |
| launchPoint = LaunchPointList::getInstance().createBootmarkByDB(appDesc, results[i]); | |
| LaunchPointList::getInstance().add(launchPoint); | |
| } | |
| } |
if type is equal to default then launchPoint is not checked. I also noticed that some functions in this class like onFind, onPutKind and onPutPermissions return always true.
The same function (getByLaunchPointId) when used in other pieces of code is checked.
Below there is an example:
sam/src/base/RunningAppList.cpp
Lines 87 to 93 in 1417869
| RunningAppPtr RunningAppList::createByLaunchPointId(const string& launchPointId) | |
| { | |
| LaunchPointPtr launchPoint = LaunchPointList::getInstance().getByLaunchPointId(launchPointId); | |
| if (launchPoint == nullptr) { | |
| Logger::warning(getClassName(), __FUNCTION__, "Cannot find launchPoint"); | |
| return nullptr; | |
| } |
I'm not sure if the check is missing because in these cases there is always a valid launchPointId. Maybe it's better to always perform the check before using the variable.