Clarify usage of smart pointers
- Members should be smart (shared or unique)
- Prefer function args to be bare, unless the arg is intended to be
stored by the callee
- Function args that are smart should be const refs to avoid an extra
copy
Change-Id: I8052fa432bcffbabff9d67a8d568640cac64d4ad
diff --git a/modules/input/evdev/EvdevModule.cpp b/modules/input/evdev/EvdevModule.cpp
index e9c8222..f6df219 100644
--- a/modules/input/evdev/EvdevModule.cpp
+++ b/modules/input/evdev/EvdevModule.cpp
@@ -47,16 +47,16 @@
InputHost mInputHost;
std::shared_ptr<InputDeviceManager> mDeviceManager;
- std::shared_ptr<InputHub> mInputHub;
+ std::unique_ptr<InputHub> mInputHub;
std::thread mPollThread;
};
-static std::shared_ptr<EvdevModule> gEvdevModule;
+static std::unique_ptr<EvdevModule> gEvdevModule;
EvdevModule::EvdevModule(InputHost inputHost) :
mInputHost(inputHost),
mDeviceManager(std::make_shared<InputDeviceManager>()),
- mInputHub(std::make_shared<InputHub>(mDeviceManager)) {}
+ mInputHub(std::make_unique<InputHub>(mDeviceManager)) {}
void EvdevModule::init() {
ALOGV("%s", __func__);
@@ -98,7 +98,7 @@
input_host_t* host, input_host_callbacks_t cb) {
LOG_ALWAYS_FATAL_IF(strcmp(module->common.id, INPUT_HARDWARE_MODULE_ID) != 0);
InputHost inputHost = {host, cb};
- gEvdevModule = std::make_shared<EvdevModule>(inputHost);
+ gEvdevModule = std::make_unique<EvdevModule>(inputHost);
gEvdevModule->init();
}
diff --git a/modules/input/evdev/InputDevice.cpp b/modules/input/evdev/InputDevice.cpp
index c0b59d7..16f8039 100644
--- a/modules/input/evdev/InputDevice.cpp
+++ b/modules/input/evdev/InputDevice.cpp
@@ -34,7 +34,7 @@
namespace android {
-EvdevDevice::EvdevDevice(std::shared_ptr<InputDeviceNode> node) :
+EvdevDevice::EvdevDevice(const std::shared_ptr<InputDeviceNode>& node) :
mDeviceNode(node) {}
void EvdevDevice::processInput(InputEvent& event, nsecs_t currentTime) {
diff --git a/modules/input/evdev/InputDevice.h b/modules/input/evdev/InputDevice.h
index 3aa16cc..7a99f90 100644
--- a/modules/input/evdev/InputDevice.h
+++ b/modules/input/evdev/InputDevice.h
@@ -43,7 +43,7 @@
*/
class EvdevDevice : public InputDeviceInterface {
public:
- explicit EvdevDevice(std::shared_ptr<InputDeviceNode> node);
+ explicit EvdevDevice(const std::shared_ptr<InputDeviceNode>& node);
virtual ~EvdevDevice() override = default;
virtual void processInput(InputEvent& event, nsecs_t currentTime) override;
diff --git a/modules/input/evdev/InputDeviceManager.cpp b/modules/input/evdev/InputDeviceManager.cpp
index ceddd90..79a9610 100644
--- a/modules/input/evdev/InputDeviceManager.cpp
+++ b/modules/input/evdev/InputDeviceManager.cpp
@@ -24,7 +24,7 @@
namespace android {
-void InputDeviceManager::onInputEvent(std::shared_ptr<InputDeviceNode> node, InputEvent& event,
+void InputDeviceManager::onInputEvent(const std::shared_ptr<InputDeviceNode>& node, InputEvent& event,
nsecs_t event_time) {
if (mDevices[node] == nullptr) {
ALOGE("got input event for unknown node %s", node->getPath().c_str());
@@ -33,11 +33,11 @@
mDevices[node]->processInput(event, event_time);
}
-void InputDeviceManager::onDeviceAdded(std::shared_ptr<InputDeviceNode> node) {
+void InputDeviceManager::onDeviceAdded(const std::shared_ptr<InputDeviceNode>& node) {
mDevices[node] = std::make_shared<EvdevDevice>(node);
}
-void InputDeviceManager::onDeviceRemoved(std::shared_ptr<InputDeviceNode> node) {
+void InputDeviceManager::onDeviceRemoved(const std::shared_ptr<InputDeviceNode>& node) {
if (mDevices[node] == nullptr) {
ALOGE("could not remove unknown node %s", node->getPath().c_str());
return;
diff --git a/modules/input/evdev/InputDeviceManager.h b/modules/input/evdev/InputDeviceManager.h
index b652155..2c0ffc8 100644
--- a/modules/input/evdev/InputDeviceManager.h
+++ b/modules/input/evdev/InputDeviceManager.h
@@ -36,10 +36,10 @@
public:
virtual ~InputDeviceManager() override = default;
- virtual void onInputEvent(std::shared_ptr<InputDeviceNode> node, InputEvent& event,
+ virtual void onInputEvent(const std::shared_ptr<InputDeviceNode>& node, InputEvent& event,
nsecs_t event_time) override;
- virtual void onDeviceAdded(std::shared_ptr<InputDeviceNode> node) override;
- virtual void onDeviceRemoved(std::shared_ptr<InputDeviceNode> node) override;
+ virtual void onDeviceAdded(const std::shared_ptr<InputDeviceNode>& node) override;
+ virtual void onDeviceRemoved(const std::shared_ptr<InputDeviceNode>& node) override;
private:
template<class T, class U>
diff --git a/modules/input/evdev/InputHub.cpp b/modules/input/evdev/InputHub.cpp
index e72ac2e..ee64b29 100644
--- a/modules/input/evdev/InputHub.cpp
+++ b/modules/input/evdev/InputHub.cpp
@@ -396,7 +396,7 @@
}
}
-InputHub::InputHub(std::shared_ptr<InputCallbackInterface> cb) :
+InputHub::InputHub(const std::shared_ptr<InputCallbackInterface>& cb) :
mInputCallback(cb) {
// Determine the type of suspend blocking we can do on this device. There
// are 3 options, in decreasing order of preference:
@@ -670,9 +670,8 @@
ALOGV("inotify event for path %s", path.c_str());
if (event->mask & IN_CREATE) {
- std::shared_ptr<InputDeviceNode> deviceNode;
- status_t res = openNode(path, &deviceNode);
- if (res != OK) {
+ auto deviceNode = openNode(path);
+ if (deviceNode == nullptr) {
ALOGE("could not open device node %s. err=%d", path.c_str(), res);
} else {
mInputCallback->onDeviceAdded(deviceNode);
@@ -680,7 +679,7 @@
} else {
auto deviceNode = findNodeByPath(path);
if (deviceNode != nullptr) {
- status_t ret = closeNode(deviceNode);
+ status_t ret = closeNode(deviceNode.get());
if (ret != OK) {
ALOGW("Could not close device %s. errno=%d", path.c_str(), ret);
} else {
@@ -712,8 +711,8 @@
continue;
}
std::string filename = path + "/" + dirent->d_name;
- std::shared_ptr<InputDeviceNode> node;
- if (openNode(filename, &node) != OK) {
+ auto node = openNode(filename);
+ if (node == nullptr) {
ALOGE("could not open device node %s", filename.c_str());
} else {
mInputCallback->onDeviceAdded(node);
@@ -723,18 +722,16 @@
return OK;
}
-status_t InputHub::openNode(const std::string& path,
- std::shared_ptr<InputDeviceNode>* outNode) {
+std::shared_ptr<InputDeviceNode> InputHub::openNode(const std::string& path) {
ALOGV("opening %s...", path.c_str());
auto evdevNode = std::shared_ptr<EvdevDeviceNode>(EvdevDeviceNode::openDeviceNode(path));
if (evdevNode == nullptr) {
- return UNKNOWN_ERROR;
+ return nullptr;
}
auto fd = evdevNode->getFd();
ALOGV("opened %s with fd %d", path.c_str(), fd);
- *outNode = std::static_pointer_cast<InputDeviceNode>(evdevNode);
- mDeviceNodes[fd] = *outNode;
+ mDeviceNodes[fd] = evdevNode;
struct epoll_event eventItem{};
eventItem.events = EPOLLIN;
if (mWakeupMechanism == WakeMechanism::EPOLL_WAKEUP) {
@@ -743,7 +740,7 @@
eventItem.data.u32 = fd;
if (epoll_ctl(mEpollFd, EPOLL_CTL_ADD, fd, &eventItem)) {
ALOGE("Could not add device fd to epoll instance. errno=%d", errno);
- return -errno;
+ return nullptr;
}
if (mNeedToCheckSuspendBlockIoctl) {
@@ -765,12 +762,12 @@
mNeedToCheckSuspendBlockIoctl = false;
}
- return OK;
+ return evdevNode;
}
-status_t InputHub::closeNode(const std::shared_ptr<InputDeviceNode>& node) {
+status_t InputHub::closeNode(const InputDeviceNode* node) {
for (auto pair : mDeviceNodes) {
- if (pair.second.get() == node.get()) {
+ if (pair.second.get() == node) {
return closeNodeByFd(pair.first);
}
}
diff --git a/modules/input/evdev/InputHub.h b/modules/input/evdev/InputHub.h
index bec327a..dfab3db 100644
--- a/modules/input/evdev/InputHub.h
+++ b/modules/input/evdev/InputHub.h
@@ -89,10 +89,10 @@
/** Callback interface for receiving input events, including device changes. */
class InputCallbackInterface {
public:
- virtual void onInputEvent(std::shared_ptr<InputDeviceNode> node, InputEvent& event,
+ virtual void onInputEvent(const std::shared_ptr<InputDeviceNode>& node, InputEvent& event,
nsecs_t event_time) = 0;
- virtual void onDeviceAdded(std::shared_ptr<InputDeviceNode> node) = 0;
- virtual void onDeviceRemoved(std::shared_ptr<InputDeviceNode> node) = 0;
+ virtual void onDeviceAdded(const std::shared_ptr<InputDeviceNode>& node) = 0;
+ virtual void onDeviceRemoved(const std::shared_ptr<InputDeviceNode>& node) = 0;
protected:
InputCallbackInterface() = default;
@@ -129,7 +129,7 @@
*/
class InputHub : public InputHubInterface {
public:
- explicit InputHub(std::shared_ptr<InputCallbackInterface> cb);
+ explicit InputHub(const std::shared_ptr<InputCallbackInterface>& cb);
virtual ~InputHub() override;
virtual status_t registerDevicePath(const std::string& path) override;
@@ -143,8 +143,8 @@
private:
status_t readNotify();
status_t scanDir(const std::string& path);
- status_t openNode(const std::string& path, std::shared_ptr<InputDeviceNode>* outNode);
- status_t closeNode(const std::shared_ptr<InputDeviceNode>& node);
+ std::shared_ptr<InputDeviceNode> openNode(const std::string& path);
+ status_t closeNode(const InputDeviceNode* node);
status_t closeNodeByFd(int fd);
std::shared_ptr<InputDeviceNode> findNodeByPath(const std::string& path);
diff --git a/tests/input/evdev/InputHub_test.cpp b/tests/input/evdev/InputHub_test.cpp
index f2c8edf..f2967b9 100644
--- a/tests/input/evdev/InputHub_test.cpp
+++ b/tests/input/evdev/InputHub_test.cpp
@@ -41,11 +41,11 @@
using namespace std::literals::chrono_literals;
-using InputCbFunc = std::function<void(std::shared_ptr<InputDeviceNode>, InputEvent&, nsecs_t)>;
-using DeviceCbFunc = std::function<void(std::shared_ptr<InputDeviceNode>)>;
+using InputCbFunc = std::function<void(const std::shared_ptr<InputDeviceNode>&, InputEvent&, nsecs_t)>;
+using DeviceCbFunc = std::function<void(const std::shared_ptr<InputDeviceNode>&)>;
-static const InputCbFunc kNoopInputCb = [](std::shared_ptr<InputDeviceNode>, InputEvent&, nsecs_t){};
-static const DeviceCbFunc kNoopDeviceCb = [](std::shared_ptr<InputDeviceNode>){};
+static const InputCbFunc kNoopInputCb = [](const std::shared_ptr<InputDeviceNode>&, InputEvent&, nsecs_t){};
+static const DeviceCbFunc kNoopDeviceCb = [](const std::shared_ptr<InputDeviceNode>&){};
class TestInputCallback : public InputCallbackInterface {
public:
@@ -57,14 +57,14 @@
void setDeviceAddedCallback(DeviceCbFunc cb) { mDeviceAddedCb = cb; }
void setDeviceRemovedCallback(DeviceCbFunc cb) { mDeviceRemovedCb = cb; }
- virtual void onInputEvent(std::shared_ptr<InputDeviceNode> node, InputEvent& event,
+ virtual void onInputEvent(const std::shared_ptr<InputDeviceNode>& node, InputEvent& event,
nsecs_t event_time) override {
mInputCb(node, event, event_time);
}
- virtual void onDeviceAdded(std::shared_ptr<InputDeviceNode> node) override {
+ virtual void onDeviceAdded(const std::shared_ptr<InputDeviceNode>& node) override {
mDeviceAddedCb(node);
}
- virtual void onDeviceRemoved(std::shared_ptr<InputDeviceNode> node) override {
+ virtual void onDeviceRemoved(const std::shared_ptr<InputDeviceNode>& node) override {
mDeviceRemovedCb(node);
}
@@ -101,7 +101,7 @@
std::string pathname;
// Expect that this callback will run and set handle and pathname.
mCallback->setDeviceAddedCallback(
- [&](std::shared_ptr<InputDeviceNode> node) {
+ [&](const std::shared_ptr<InputDeviceNode>& node) {
pathname = node->getPath();
});
@@ -136,11 +136,11 @@
std::shared_ptr<InputDeviceNode> tempNode;
// Expect that these callbacks will run for the above device file.
mCallback->setDeviceAddedCallback(
- [&](std::shared_ptr<InputDeviceNode> node) {
+ [&](const std::shared_ptr<InputDeviceNode>& node) {
tempNode = node;
});
mCallback->setDeviceRemovedCallback(
- [&](std::shared_ptr<InputDeviceNode> node) {
+ [&](const std::shared_ptr<InputDeviceNode>& node) {
EXPECT_EQ(tempNode, node);
});
@@ -182,7 +182,8 @@
// Expect this callback to run when the input event is read.
nsecs_t expectedWhen = systemTime(CLOCK_MONOTONIC) + ms2ns(inputDelayMs.count());
mCallback->setInputCallback(
- [&](std::shared_ptr<InputDeviceNode> node, InputEvent& event, nsecs_t event_time) {
+ [&](const std::shared_ptr<InputDeviceNode>& node, InputEvent& event,
+ nsecs_t event_time) {
EXPECT_NEAR(expectedWhen, event_time, ms2ns(TIMING_TOLERANCE_MS));
EXPECT_EQ(s2ns(1), event.when);
EXPECT_EQ(tempFileName, node->getPath());
@@ -211,7 +212,7 @@
// Setup the callback for input events. Should run before the device
// callback.
mCallback->setInputCallback(
- [&](std::shared_ptr<InputDeviceNode>, InputEvent&, nsecs_t) {
+ [&](const std::shared_ptr<InputDeviceNode>&, InputEvent&, nsecs_t) {
ASSERT_FALSE(deviceCallbackFinished);
inputCallbackFinished = true;
});
@@ -219,7 +220,7 @@
// Setup the callback for device removal. Should run after the input
// callback.
mCallback->setDeviceRemovedCallback(
- [&](std::shared_ptr<InputDeviceNode> node) {
+ [&](const std::shared_ptr<InputDeviceNode>& node) {
ASSERT_TRUE(inputCallbackFinished)
<< "input callback did not run before device changed callback";
// Make sure the correct device was removed.