Return a copy of InputDeviceLightInfo instead of pointer
Refactor InputReaderInterface to return a copy instead of pointer. This
will ensure that we don't read from memory that's been freed.
Also, refactor InputDevice api's to return a list of infos instead of
having to query each individually. In all usages so far, there's no need
to get a specific info by type.
Bug: 190126442
Test: atest inputflinger_tests libinput_tests
Change-Id: I7f993a14259bb802e2631663c1c8bb65cc9b6702
diff --git a/include/input/InputDevice.h b/include/input/InputDevice.h
index 1fec080..7f0324a 100644
--- a/include/input/InputDevice.h
+++ b/include/input/InputDevice.h
@@ -257,13 +257,9 @@
return mMotionRanges;
}
- const InputDeviceSensorInfo* getSensorInfo(InputDeviceSensorType type);
+ std::vector<InputDeviceSensorInfo> getSensors();
- const std::vector<InputDeviceSensorType> getSensorTypes();
-
- const std::vector<int32_t> getLightIds();
-
- const InputDeviceLightInfo* getLightInfo(int32_t id);
+ std::vector<InputDeviceLightInfo> getLights();
private:
int32_t mId;
diff --git a/libs/input/InputDevice.cpp b/libs/input/InputDevice.cpp
index 61d72ad..30c42a3 100644
--- a/libs/input/InputDevice.cpp
+++ b/libs/input/InputDevice.cpp
@@ -247,36 +247,22 @@
mLights.insert_or_assign(info.id, info);
}
-const std::vector<InputDeviceSensorType> InputDeviceInfo::getSensorTypes() {
- std::vector<InputDeviceSensorType> types;
+std::vector<InputDeviceSensorInfo> InputDeviceInfo::getSensors() {
+ std::vector<InputDeviceSensorInfo> infos;
+ infos.reserve(mSensors.size());
for (const auto& [type, info] : mSensors) {
- types.push_back(type);
+ infos.push_back(info);
}
- return types;
+ return infos;
}
-const InputDeviceSensorInfo* InputDeviceInfo::getSensorInfo(InputDeviceSensorType type) {
- auto it = mSensors.find(type);
- if (it == mSensors.end()) {
- return nullptr;
- }
- return &it->second;
-}
-
-const std::vector<int32_t> InputDeviceInfo::getLightIds() {
- std::vector<int32_t> ids;
+std::vector<InputDeviceLightInfo> InputDeviceInfo::getLights() {
+ std::vector<InputDeviceLightInfo> infos;
+ infos.reserve(mLights.size());
for (const auto& [id, info] : mLights) {
- ids.push_back(id);
+ infos.push_back(info);
}
- return ids;
-}
-
-const InputDeviceLightInfo* InputDeviceInfo::getLightInfo(int32_t id) {
- auto it = mLights.find(id);
- if (it == mLights.end()) {
- return nullptr;
- }
- return &it->second;
+ return infos;
}
} // namespace android
diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h
index 19abfd9..7fdbbfd 100644
--- a/services/inputflinger/include/InputReaderBase.h
+++ b/services/inputflinger/include/InputReaderBase.h
@@ -113,9 +113,9 @@
/* Get battery status of a particular input device. */
virtual std::optional<int32_t> getBatteryStatus(int32_t deviceId) = 0;
- virtual std::vector<int32_t> getLightIds(int32_t deviceId) = 0;
+ virtual std::vector<InputDeviceLightInfo> getLights(int32_t deviceId) = 0;
- virtual const InputDeviceLightInfo* getLightInfo(int32_t deviceId, int32_t lightId) = 0;
+ virtual std::vector<InputDeviceSensorInfo> getSensors(int32_t deviceId) = 0;
/* Return true if the device can send input events to the specified display. */
virtual bool canDispatchToDisplay(int32_t deviceId, int32_t displayId) = 0;
diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
index ad503fd..7af014c 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -89,8 +89,7 @@
}
void InputDevice::dump(std::string& dump, const std::string& eventHubDevStr) {
- InputDeviceInfo deviceInfo;
- getDeviceInfo(&deviceInfo);
+ InputDeviceInfo deviceInfo = getDeviceInfo();
dump += StringPrintf(INDENT "Device %d: %s\n", deviceInfo.getId(),
deviceInfo.getDisplayName().c_str());
@@ -417,15 +416,17 @@
for_each_mapper([state](InputMapper& mapper) { mapper.updateExternalStylusState(state); });
}
-void InputDevice::getDeviceInfo(InputDeviceInfo* outDeviceInfo) {
- outDeviceInfo->initialize(mId, mGeneration, mControllerNumber, mIdentifier, mAlias, mIsExternal,
- mHasMic);
+InputDeviceInfo InputDevice::getDeviceInfo() {
+ InputDeviceInfo outDeviceInfo;
+ outDeviceInfo.initialize(mId, mGeneration, mControllerNumber, mIdentifier, mAlias, mIsExternal,
+ mHasMic);
for_each_mapper(
- [outDeviceInfo](InputMapper& mapper) { mapper.populateDeviceInfo(outDeviceInfo); });
+ [&outDeviceInfo](InputMapper& mapper) { mapper.populateDeviceInfo(&outDeviceInfo); });
if (mController) {
- mController->populateDeviceInfo(outDeviceInfo);
+ mController->populateDeviceInfo(&outDeviceInfo);
}
+ return outDeviceInfo;
}
int32_t InputDevice::getKeyCodeState(uint32_t sourceMask, int32_t keyCode) {
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index e91f84e..10c04f6 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -406,9 +406,7 @@
for (auto& devicePair : mDevices) {
std::shared_ptr<InputDevice>& device = devicePair.second;
if (device->getClasses().test(InputDeviceClass::EXTERNAL_STYLUS) && !device->isIgnored()) {
- InputDeviceInfo info;
- device->getDeviceInfo(&info);
- outDevices.push_back(info);
+ outDevices.push_back(device->getDeviceInfo());
}
}
}
@@ -498,9 +496,7 @@
for (const auto& [device, eventHubIds] : mDeviceToEventHubIdsMap) {
if (!device->isIgnored()) {
- InputDeviceInfo info;
- device->getDeviceInfo(&info);
- outInputDevices.push_back(info);
+ outInputDevices.push_back(device->getDeviceInfo());
}
}
return outInputDevices;
@@ -695,28 +691,26 @@
return std::nullopt;
}
-std::vector<int32_t> InputReader::getLightIds(int32_t deviceId) {
+std::vector<InputDeviceLightInfo> InputReader::getLights(int32_t deviceId) {
std::scoped_lock _l(mLock);
InputDevice* device = findInputDeviceLocked(deviceId);
- if (device) {
- InputDeviceInfo info;
- device->getDeviceInfo(&info);
- return info.getLightIds();
+ if (device == nullptr) {
+ return {};
}
- return {};
+
+ return device->getDeviceInfo().getLights();
}
-const InputDeviceLightInfo* InputReader::getLightInfo(int32_t deviceId, int32_t lightId) {
+std::vector<InputDeviceSensorInfo> InputReader::getSensors(int32_t deviceId) {
std::scoped_lock _l(mLock);
InputDevice* device = findInputDeviceLocked(deviceId);
- if (device) {
- InputDeviceInfo info;
- device->getDeviceInfo(&info);
- return info.getLightInfo(lightId);
+ if (device == nullptr) {
+ return {};
}
- return nullptr;
+
+ return device->getDeviceInfo().getSensors();
}
bool InputReader::setLightColor(int32_t deviceId, int32_t lightId, int32_t color) {
diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h
index 291f105..2f2eba7 100644
--- a/services/inputflinger/reader/include/InputDevice.h
+++ b/services/inputflinger/reader/include/InputDevice.h
@@ -80,7 +80,7 @@
void timeoutExpired(nsecs_t when);
void updateExternalStylusState(const StylusState& state);
- void getDeviceInfo(InputDeviceInfo* outDeviceInfo);
+ InputDeviceInfo getDeviceInfo();
int32_t getKeyCodeState(uint32_t sourceMask, int32_t keyCode);
int32_t getScanCodeState(uint32_t sourceMask, int32_t scanCode);
int32_t getSwitchState(uint32_t sourceMask, int32_t switchCode);
diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h
index bc79ccf..a00c5af 100644
--- a/services/inputflinger/reader/include/InputReader.h
+++ b/services/inputflinger/reader/include/InputReader.h
@@ -99,9 +99,9 @@
std::optional<int32_t> getBatteryStatus(int32_t deviceId) override;
- std::vector<int32_t> getLightIds(int32_t deviceId) override;
+ std::vector<InputDeviceLightInfo> getLights(int32_t deviceId) override;
- const InputDeviceLightInfo* getLightInfo(int32_t deviceId, int32_t lightId) override;
+ std::vector<InputDeviceSensorInfo> getSensors(int32_t deviceId) override;
bool setLightColor(int32_t deviceId, int32_t lightId, int32_t color) override;
@@ -130,24 +130,24 @@
// lock is already held by the input loop
void updateGlobalMetaState() NO_THREAD_SAFETY_ANALYSIS override;
int32_t getGlobalMetaState() NO_THREAD_SAFETY_ANALYSIS override;
- void disableVirtualKeysUntil(nsecs_t time) NO_THREAD_SAFETY_ANALYSIS override;
- bool shouldDropVirtualKey(nsecs_t now, int32_t keyCode,
- int32_t scanCode) NO_THREAD_SAFETY_ANALYSIS override;
- void fadePointer() NO_THREAD_SAFETY_ANALYSIS override;
+ void disableVirtualKeysUntil(nsecs_t time) REQUIRES(mReader->mLock) override;
+ bool shouldDropVirtualKey(nsecs_t now, int32_t keyCode, int32_t scanCode)
+ REQUIRES(mReader->mLock) override;
+ void fadePointer() REQUIRES(mReader->mLock) override;
std::shared_ptr<PointerControllerInterface> getPointerController(int32_t deviceId)
- NO_THREAD_SAFETY_ANALYSIS override;
- void requestTimeoutAtTime(nsecs_t when) NO_THREAD_SAFETY_ANALYSIS override;
+ REQUIRES(mReader->mLock) override;
+ void requestTimeoutAtTime(nsecs_t when) REQUIRES(mReader->mLock) override;
int32_t bumpGeneration() NO_THREAD_SAFETY_ANALYSIS override;
void getExternalStylusDevices(std::vector<InputDeviceInfo>& outDevices)
- NO_THREAD_SAFETY_ANALYSIS override;
+ REQUIRES(mReader->mLock) override;
void dispatchExternalStylusState(const StylusState& outState)
- NO_THREAD_SAFETY_ANALYSIS override;
- InputReaderPolicyInterface* getPolicy() NO_THREAD_SAFETY_ANALYSIS override;
- InputListenerInterface* getListener() NO_THREAD_SAFETY_ANALYSIS override;
- EventHubInterface* getEventHub() NO_THREAD_SAFETY_ANALYSIS override;
+ REQUIRES(mReader->mLock) override;
+ InputReaderPolicyInterface* getPolicy() REQUIRES(mReader->mLock) override;
+ InputListenerInterface* getListener() REQUIRES(mReader->mLock) override;
+ EventHubInterface* getEventHub() REQUIRES(mReader->mLock) override;
int32_t getNextId() NO_THREAD_SAFETY_ANALYSIS override;
- void updateLedMetaState(int32_t metaState) NO_THREAD_SAFETY_ANALYSIS override;
- int32_t getLedMetaState() NO_THREAD_SAFETY_ANALYSIS override;
+ void updateLedMetaState(int32_t metaState) REQUIRES(mReader->mLock) override;
+ int32_t getLedMetaState() REQUIRES(mReader->mLock) REQUIRES(mLock) override;
} mContext;
friend class ContextImpl;
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 7a11ca7..73198bc 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -1524,20 +1524,6 @@
}
};
-TEST_F(InputReaderTest, ReaderGetInputDevices) {
- ASSERT_NO_FATAL_FAILURE(addDevice(1, "keyboard", InputDeviceClass::KEYBOARD, nullptr));
- ASSERT_NO_FATAL_FAILURE(addDevice(2, "ignored", Flags<InputDeviceClass>(0),
- nullptr)); // no classes so device will be ignored
-
- const std::vector<InputDeviceInfo> inputDevices = mReader->getInputDevices();
- ASSERT_EQ(1U, inputDevices.size());
- ASSERT_EQ(END_RESERVED_ID + 1, inputDevices[0].getId());
- ASSERT_STREQ("keyboard", inputDevices[0].getIdentifier().name.c_str());
- ASSERT_EQ(AINPUT_KEYBOARD_TYPE_NON_ALPHABETIC, inputDevices[0].getKeyboardType());
- ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, inputDevices[0].getSources());
- ASSERT_EQ(size_t(0), inputDevices[0].getMotionRanges().size());
-}
-
TEST_F(InputReaderTest, PolicyGetInputDevices) {
ASSERT_NO_FATAL_FAILURE(addDevice(1, "keyboard", InputDeviceClass::KEYBOARD, nullptr));
ASSERT_NO_FATAL_FAILURE(addDevice(2, "ignored", Flags<InputDeviceClass>(0),
@@ -1550,7 +1536,7 @@
ASSERT_STREQ("keyboard", inputDevices[0].getIdentifier().name.c_str());
ASSERT_EQ(AINPUT_KEYBOARD_TYPE_NON_ALPHABETIC, inputDevices[0].getKeyboardType());
ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, inputDevices[0].getSources());
- ASSERT_EQ(size_t(0), inputDevices[0].getMotionRanges().size());
+ ASSERT_EQ(0U, inputDevices[0].getMotionRanges().size());
}
TEST_F(InputReaderTest, GetMergedInputDevices) {
@@ -1571,7 +1557,7 @@
addDevice(eventHubIds[1], "fake2", InputDeviceClass::KEYBOARD, nullptr));
// Two devices will be merged to one input device as they have same identifier
- ASSERT_EQ(1U, mReader->getInputDevices().size());
+ ASSERT_EQ(1U, mFakePolicy->getInputDevices().size());
}
TEST_F(InputReaderTest, GetMergedInputDevicesEnabled) {
@@ -2189,7 +2175,7 @@
ASSERT_EQ(initialNumDevices + 1, mFakePolicy->getInputDevices().size());
// Find the test device by its name.
- const std::vector<InputDeviceInfo> inputDevices = mReader->getInputDevices();
+ const std::vector<InputDeviceInfo> inputDevices = mFakePolicy->getInputDevices();
const auto& it =
std::find_if(inputDevices.begin(), inputDevices.end(),
[&keyboard](const InputDeviceInfo& info) {
@@ -2463,8 +2449,7 @@
ASSERT_TRUE(mDevice->isIgnored());
ASSERT_EQ(AINPUT_SOURCE_UNKNOWN, mDevice->getSources());
- InputDeviceInfo info;
- mDevice->getDeviceInfo(&info);
+ InputDeviceInfo info = mDevice->getDeviceInfo();
ASSERT_EQ(DEVICE_ID, info.getId());
ASSERT_STREQ(DEVICE_NAME, info.getIdentifier().name.c_str());
ASSERT_EQ(AINPUT_KEYBOARD_TYPE_NONE, info.getKeyboardType());
@@ -2533,8 +2518,7 @@
ASSERT_FALSE(mDevice->isIgnored());
ASSERT_EQ(uint32_t(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_TOUCHSCREEN), mDevice->getSources());
- InputDeviceInfo info;
- mDevice->getDeviceInfo(&info);
+ InputDeviceInfo info = mDevice->getDeviceInfo();
ASSERT_EQ(DEVICE_ID, info.getId());
ASSERT_STREQ(DEVICE_NAME, info.getIdentifier().name.c_str());
ASSERT_EQ(AINPUT_KEYBOARD_TYPE_ALPHABETIC, info.getKeyboardType());
@@ -8444,8 +8428,7 @@
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
ASSERT_EQ(AINPUT_SOURCE_TOUCHPAD, mapper.getSources());
- InputDeviceInfo deviceInfo;
- mDevice->getDeviceInfo(&deviceInfo);
+ InputDeviceInfo deviceInfo = mDevice->getDeviceInfo();
const InputDeviceInfo::MotionRange* relRangeX =
deviceInfo.getMotionRange(AMOTION_EVENT_AXIS_RELATIVE_X, AINPUT_SOURCE_TOUCHPAD);
@@ -8755,12 +8738,12 @@
PeripheralController& controller = addControllerAndConfigure<PeripheralController>();
InputDeviceInfo info;
controller.populateDeviceInfo(&info);
- const auto& ids = info.getLightIds();
- ASSERT_EQ(1UL, ids.size());
- ASSERT_EQ(InputDeviceLightType::MONO, info.getLightInfo(ids[0])->type);
+ std::vector<InputDeviceLightInfo> lights = info.getLights();
+ ASSERT_EQ(1U, lights.size());
+ ASSERT_EQ(InputDeviceLightType::MONO, lights[0].type);
- ASSERT_TRUE(controller.setLightColor(ids[0], LIGHT_BRIGHTNESS));
- ASSERT_EQ(controller.getLightColor(ids[0]).value_or(-1), LIGHT_BRIGHTNESS);
+ ASSERT_TRUE(controller.setLightColor(lights[0].id, LIGHT_BRIGHTNESS));
+ ASSERT_EQ(controller.getLightColor(lights[0].id).value_or(-1), LIGHT_BRIGHTNESS);
}
TEST_F(LightControllerTest, RGBLight) {
@@ -8786,12 +8769,12 @@
PeripheralController& controller = addControllerAndConfigure<PeripheralController>();
InputDeviceInfo info;
controller.populateDeviceInfo(&info);
- const auto& ids = info.getLightIds();
- ASSERT_EQ(1UL, ids.size());
- ASSERT_EQ(InputDeviceLightType::RGB, info.getLightInfo(ids[0])->type);
+ std::vector<InputDeviceLightInfo> lights = info.getLights();
+ ASSERT_EQ(1U, lights.size());
+ ASSERT_EQ(InputDeviceLightType::RGB, lights[0].type);
- ASSERT_TRUE(controller.setLightColor(ids[0], LIGHT_COLOR));
- ASSERT_EQ(controller.getLightColor(ids[0]).value_or(-1), LIGHT_COLOR);
+ ASSERT_TRUE(controller.setLightColor(lights[0].id, LIGHT_COLOR));
+ ASSERT_EQ(controller.getLightColor(lights[0].id).value_or(-1), LIGHT_COLOR);
}
TEST_F(LightControllerTest, MultiColorRGBLight) {
@@ -8808,12 +8791,12 @@
PeripheralController& controller = addControllerAndConfigure<PeripheralController>();
InputDeviceInfo info;
controller.populateDeviceInfo(&info);
- const auto& ids = info.getLightIds();
- ASSERT_EQ(1UL, ids.size());
- ASSERT_EQ(InputDeviceLightType::MULTI_COLOR, info.getLightInfo(ids[0])->type);
+ std::vector<InputDeviceLightInfo> lights = info.getLights();
+ ASSERT_EQ(1U, lights.size());
+ ASSERT_EQ(InputDeviceLightType::MULTI_COLOR, lights[0].type);
- ASSERT_TRUE(controller.setLightColor(ids[0], LIGHT_COLOR));
- ASSERT_EQ(controller.getLightColor(ids[0]).value_or(-1), LIGHT_COLOR);
+ ASSERT_TRUE(controller.setLightColor(lights[0].id, LIGHT_COLOR));
+ ASSERT_EQ(controller.getLightColor(lights[0].id).value_or(-1), LIGHT_COLOR);
}
TEST_F(LightControllerTest, PlayerIdLight) {
@@ -8845,13 +8828,13 @@
PeripheralController& controller = addControllerAndConfigure<PeripheralController>();
InputDeviceInfo info;
controller.populateDeviceInfo(&info);
- const auto& ids = info.getLightIds();
- ASSERT_EQ(1UL, ids.size());
- ASSERT_EQ(InputDeviceLightType::PLAYER_ID, info.getLightInfo(ids[0])->type);
+ std::vector<InputDeviceLightInfo> lights = info.getLights();
+ ASSERT_EQ(1U, lights.size());
+ ASSERT_EQ(InputDeviceLightType::PLAYER_ID, lights[0].type);
- ASSERT_FALSE(controller.setLightColor(ids[0], LIGHT_COLOR));
- ASSERT_TRUE(controller.setLightPlayerId(ids[0], LIGHT_PLAYER_ID));
- ASSERT_EQ(controller.getLightPlayerId(ids[0]).value_or(-1), LIGHT_PLAYER_ID);
+ ASSERT_FALSE(controller.setLightColor(lights[0].id, LIGHT_COLOR));
+ ASSERT_TRUE(controller.setLightPlayerId(lights[0].id, LIGHT_PLAYER_ID));
+ ASSERT_EQ(controller.getLightPlayerId(lights[0].id).value_or(-1), LIGHT_PLAYER_ID);
}
} // namespace android