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 (file)
--- a/modules/input/evdev/EvdevModule.cpp
+++ b/modules/input/evdev/EvdevModule.cpp
@@ -47,16 +47,16 @@ private:
 
     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 @@ static void input_init(const input_module_t* module,
         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 (file)
--- 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 (file)
--- a/modules/input/evdev/InputDevice.h
+++ b/modules/input/evdev/InputDevice.h
@@ -43,7 +43,7 @@ protected:
  */
 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 (file)
--- 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 @@ void InputDeviceManager::onInputEvent(std::shared_ptr<InputDeviceNode> node, Inp
     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 (file)
--- a/modules/input/evdev/InputDeviceManager.h
+++ b/modules/input/evdev/InputDeviceManager.h
@@ -36,10 +36,10 @@ class InputDeviceManager : public InputCallbackInterface {
 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 (file)
--- a/modules/input/evdev/InputHub.cpp
+++ b/modules/input/evdev/InputHub.cpp
@@ -396,7 +396,7 @@ void EvdevDeviceNode::disableDriverKeyRepeat() {
     }
 }
 
-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 @@ status_t InputHub::readNotify() {
             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 @@ status_t InputHub::readNotify() {
             } 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 @@ status_t InputHub::scanDir(const std::string& path) {
             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 @@ status_t InputHub::scanDir(const std::string& path) {
     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 @@ status_t InputHub::openNode(const std::string& path,
     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 @@ status_t InputHub::openNode(const std::string& path,
         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 (file)
--- a/modules/input/evdev/InputHub.h
+++ b/modules/input/evdev/InputHub.h
@@ -89,10 +89,10 @@ protected:
 /** 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 @@ protected:
  */
 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 @@ public:
 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 (file)
--- a/tests/input/evdev/InputHub_test.cpp
+++ b/tests/input/evdev/InputHub_test.cpp
@@ -41,11 +41,11 @@ namespace tests {
 
 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 @@ public:
     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 @@ TEST_F(InputHubTest, DISABLED_testDeviceAdded) {
     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 @@ TEST_F(InputHubTest, DISABLED_testDeviceRemoved) {
     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 @@ TEST_F(InputHubTest, DISABLED_testInputEvent) {
     // 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 @@ TEST_F(InputHubTest, DISABLED_testCallbackOrder) {
     // 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 @@ TEST_F(InputHubTest, DISABLED_testCallbackOrder) {
     // 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.