From 5ed8eaab67ad7ec3ac1110d696f5d89596a5a887 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 18 May 2022 12:30:16 -0700 Subject: Refactor input code for require_kernel_config parameter As part of the next commit to allow kl files to specify a required kernel config, some small refactorings were done. Move those here to a separate CL to make it easier to review. Bug: 228005926 Test: atest libinput_tests Change-Id: Iab06bb6ef44807308ee2b3e6b8a0c780bb748f09 --- libs/input/KeyLayoutMap.cpp | 115 +++++++++++++++++++------------------------- 1 file changed, 49 insertions(+), 66 deletions(-) (limited to 'libs/input/KeyLayoutMap.cpp') diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index 7c25cda9ac..17c3bb36e3 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -21,8 +21,8 @@ #include #include #include +#include #include -#include #include #include @@ -30,15 +30,22 @@ #include #include -// Enables debug output for the parser. -#define DEBUG_PARSER 0 +/** + * Log debug output for the parser. + * Enable this via "adb shell setprop log.tag.KeyLayoutMapParser DEBUG" (requires restart) + */ +const bool DEBUG_PARSER = + __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG "Parser", ANDROID_LOG_INFO); // Enables debug output for parser performance. #define DEBUG_PARSER_PERFORMANCE 0 -// Enables debug output for mapping. -#define DEBUG_MAPPING 0 - +/** + * Log debug output for mapping. + * Enable this via "adb shell setprop log.tag.KeyLayoutMapMapping DEBUG" (requires restart) + */ +const bool DEBUG_MAPPING = + __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG "Mapping", ANDROID_LOG_INFO); namespace android { namespace { @@ -134,9 +141,8 @@ status_t KeyLayoutMap::mapKey(int32_t scanCode, int32_t usageCode, int32_t* outKeyCode, uint32_t* outFlags) const { const Key* key = getKey(scanCode, usageCode); if (!key) { -#if DEBUG_MAPPING - ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Failed.", scanCode, usageCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "mapKey: scanCode=%d, usageCode=0x%08x ~ Failed.", scanCode, + usageCode); *outKeyCode = AKEYCODE_UNKNOWN; *outFlags = 0; return NAME_NOT_FOUND; @@ -145,10 +151,9 @@ status_t KeyLayoutMap::mapKey(int32_t scanCode, int32_t usageCode, *outKeyCode = key->keyCode; *outFlags = key->flags; -#if DEBUG_MAPPING - ALOGD("mapKey: scanCode=%d, usageCode=0x%08x ~ Result keyCode=%d, outFlags=0x%08x.", - scanCode, usageCode, *outKeyCode, *outFlags); -#endif + ALOGD_IF(DEBUG_MAPPING, + "mapKey: scanCode=%d, usageCode=0x%08x ~ Result keyCode=%d, outFlags=0x%08x.", + scanCode, usageCode, *outKeyCode, *outFlags); return NO_ERROR; } @@ -156,17 +161,12 @@ status_t KeyLayoutMap::mapKey(int32_t scanCode, int32_t usageCode, base::Result> KeyLayoutMap::mapSensor(int32_t absCode) { auto it = mSensorsByAbsCode.find(absCode); if (it == mSensorsByAbsCode.end()) { -#if DEBUG_MAPPING - ALOGD("mapSensor: absCode=%d, ~ Failed.", absCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "mapSensor: absCode=%d, ~ Failed.", absCode); return Errorf("Can't find abs code {}.", absCode); } const Sensor& sensor = it->second; - -#if DEBUG_MAPPING - ALOGD("mapSensor: absCode=%d, sensorType=%s, sensorDataIndex=0x%x.", absCode, - ftl::enum_string(sensor.sensorType).c_str(), sensor.sensorDataIndex); -#endif + ALOGD_IF(DEBUG_MAPPING, "mapSensor: absCode=%d, sensorType=%s, sensorDataIndex=0x%x.", absCode, + ftl::enum_string(sensor.sensorType).c_str(), sensor.sensorDataIndex); return std::make_pair(sensor.sensorType, sensor.sensorDataIndex); } @@ -200,21 +200,18 @@ status_t KeyLayoutMap::findScanCodesForKey( status_t KeyLayoutMap::mapAxis(int32_t scanCode, AxisInfo* outAxisInfo) const { ssize_t index = mAxes.indexOfKey(scanCode); if (index < 0) { -#if DEBUG_MAPPING - ALOGD("mapAxis: scanCode=%d ~ Failed.", scanCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "mapAxis: scanCode=%d ~ Failed.", scanCode); return NAME_NOT_FOUND; } *outAxisInfo = mAxes.valueAt(index); -#if DEBUG_MAPPING - ALOGD("mapAxis: scanCode=%d ~ Result mode=%d, axis=%d, highAxis=%d, " - "splitValue=%d, flatOverride=%d.", - scanCode, - outAxisInfo->mode, outAxisInfo->axis, outAxisInfo->highAxis, - outAxisInfo->splitValue, outAxisInfo->flatOverride); -#endif + ALOGD_IF(DEBUG_MAPPING, + "mapAxis: scanCode=%d ~ Result mode=%d, axis=%d, highAxis=%d, " + "splitValue=%d, flatOverride=%d.", + scanCode, outAxisInfo->mode, outAxisInfo->axis, outAxisInfo->highAxis, + outAxisInfo->splitValue, outAxisInfo->flatOverride); + return NO_ERROR; } @@ -223,15 +220,12 @@ status_t KeyLayoutMap::findScanCodeForLed(int32_t ledCode, int32_t* outScanCode) for (size_t i = 0; i < N; i++) { if (mLedsByScanCode.valueAt(i).ledCode == ledCode) { *outScanCode = mLedsByScanCode.keyAt(i); -#if DEBUG_MAPPING - ALOGD("findScanCodeForLed: ledCode=%d, scanCode=%d.", ledCode, *outScanCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "findScanCodeForLed: ledCode=%d, scanCode=%d.", ledCode, + *outScanCode); return NO_ERROR; } } -#if DEBUG_MAPPING - ALOGD("findScanCodeForLed: ledCode=%d ~ Not found.", ledCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "findScanCodeForLed: ledCode=%d ~ Not found.", ledCode); return NAME_NOT_FOUND; } @@ -240,15 +234,12 @@ status_t KeyLayoutMap::findUsageCodeForLed(int32_t ledCode, int32_t* outUsageCod for (size_t i = 0; i < N; i++) { if (mLedsByUsageCode.valueAt(i).ledCode == ledCode) { *outUsageCode = mLedsByUsageCode.keyAt(i); -#if DEBUG_MAPPING - ALOGD("findUsageForLed: ledCode=%d, usage=%x.", ledCode, *outUsageCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d, usage=%x.", __func__, ledCode, *outUsageCode); return NO_ERROR; } } -#if DEBUG_MAPPING - ALOGD("findUsageForLed: ledCode=%d ~ Not found.", ledCode); -#endif + ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d ~ Not found.", __func__, ledCode); + return NAME_NOT_FOUND; } @@ -264,10 +255,8 @@ KeyLayoutMap::Parser::~Parser() { status_t KeyLayoutMap::Parser::parse() { while (!mTokenizer->isEof()) { -#if DEBUG_PARSER - ALOGD("Parsing %s: '%s'.", mTokenizer->getLocation().string(), - mTokenizer->peekRemainderOfLine().string()); -#endif + ALOGD_IF(DEBUG_PARSER, "Parsing %s: '%s'.", mTokenizer->getLocation().string(), + mTokenizer->peekRemainderOfLine().string()); mTokenizer->skipDelimiters(WHITESPACE); @@ -361,10 +350,9 @@ status_t KeyLayoutMap::Parser::parseKey() { flags |= flag; } -#if DEBUG_PARSER - ALOGD("Parsed key %s: code=%d, keyCode=%d, flags=0x%08x.", - mapUsage ? "usage" : "scan code", code, keyCode, flags); -#endif + ALOGD_IF(DEBUG_PARSER, "Parsed key %s: code=%d, keyCode=%d, flags=0x%08x.", + mapUsage ? "usage" : "scan code", code, keyCode, flags); + Key key; key.keyCode = keyCode; key.flags = flags; @@ -462,13 +450,12 @@ status_t KeyLayoutMap::Parser::parseAxis() { } } -#if DEBUG_PARSER - ALOGD("Parsed axis: scanCode=%d, mode=%d, axis=%d, highAxis=%d, " - "splitValue=%d, flatOverride=%d.", - scanCode, - axisInfo.mode, axisInfo.axis, axisInfo.highAxis, - axisInfo.splitValue, axisInfo.flatOverride); -#endif + ALOGD_IF(DEBUG_PARSER, + "Parsed axis: scanCode=%d, mode=%d, axis=%d, highAxis=%d, " + "splitValue=%d, flatOverride=%d.", + scanCode, axisInfo.mode, axisInfo.axis, axisInfo.highAxis, axisInfo.splitValue, + axisInfo.flatOverride); + mMap->mAxes.add(scanCode, axisInfo); return NO_ERROR; } @@ -505,10 +492,8 @@ status_t KeyLayoutMap::Parser::parseLed() { return BAD_VALUE; } -#if DEBUG_PARSER - ALOGD("Parsed led %s: code=%d, ledCode=%d.", - mapUsage ? "usage" : "scan code", code, ledCode); -#endif + ALOGD_IF(DEBUG_PARSER, "Parsed led %s: code=%d, ledCode=%d.", mapUsage ? "usage" : "scan code", + code, ledCode); Led led; led.ledCode = ledCode; @@ -584,10 +569,8 @@ status_t KeyLayoutMap::Parser::parseSensor() { } int32_t sensorDataIndex = indexOpt.value(); -#if DEBUG_PARSER - ALOGD("Parsed sensor: abs code=%d, sensorType=%s, sensorDataIndex=%d.", code, - ftl::enum_string(sensorType).c_str(), sensorDataIndex); -#endif + ALOGD_IF(DEBUG_PARSER, "Parsed sensor: abs code=%d, sensorType=%s, sensorDataIndex=%d.", code, + ftl::enum_string(sensorType).c_str(), sensorDataIndex); Sensor sensor; sensor.sensorType = sensorType; -- cgit v1.2.3-59-g8ed1b From ef1564ca62b1b079278c1913cf57cea10743a352 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 18 May 2022 09:45:54 -0700 Subject: Avoid using KeyedVector in KeyLayoutMap There's a supported alternative, std::unordered_map, that we should be using instead. Bug: 228005926 Test: atest libinput_tests inputflinger_tests Change-Id: Ia1f41d17d7ee912534edffe1a1e7655866fa54c6 --- include/input/KeyLayoutMap.h | 19 +++---- libs/input/KeyLayoutMap.cpp | 95 ++++++++++++++----------------- services/inputflinger/reader/EventHub.cpp | 55 ++++++++---------- 3 files changed, 77 insertions(+), 92 deletions(-) (limited to 'libs/input/KeyLayoutMap.cpp') diff --git a/include/input/KeyLayoutMap.h b/include/input/KeyLayoutMap.h index d1925f4eee..006c0684e3 100644 --- a/include/input/KeyLayoutMap.h +++ b/include/input/KeyLayoutMap.h @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -70,11 +69,11 @@ public: status_t mapKey(int32_t scanCode, int32_t usageCode, int32_t* outKeyCode, uint32_t* outFlags) const; - status_t findScanCodesForKey(int32_t keyCode, std::vector* outScanCodes) const; - status_t findScanCodeForLed(int32_t ledCode, int32_t* outScanCode) const; - status_t findUsageCodeForLed(int32_t ledCode, int32_t* outUsageCode) const; + std::vector findScanCodesForKey(int32_t keyCode) const; + std::optional findScanCodeForLed(int32_t ledCode) const; + std::optional findUsageCodeForLed(int32_t ledCode) const; - status_t mapAxis(int32_t scanCode, AxisInfo* outAxisInfo) const; + std::optional mapAxis(int32_t scanCode) const; const std::string getLoadFileName() const; // Return pair of sensor type and sensor data index, for the input device abs code base::Result> mapSensor(int32_t absCode); @@ -98,11 +97,11 @@ private: int32_t sensorDataIndex; }; - KeyedVector mKeysByScanCode; - KeyedVector mKeysByUsageCode; - KeyedVector mAxes; - KeyedVector mLedsByScanCode; - KeyedVector mLedsByUsageCode; + std::unordered_map mKeysByScanCode; + std::unordered_map mKeysByUsageCode; + std::unordered_map mAxes; + std::unordered_map mLedsByScanCode; + std::unordered_map mLedsByUsageCode; std::unordered_map mSensorsByAbsCode; std::string mLoadFileName; diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index 17c3bb36e3..00fc051cea 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -172,78 +172,68 @@ base::Result> KeyLayoutMap::mapSensor( const KeyLayoutMap::Key* KeyLayoutMap::getKey(int32_t scanCode, int32_t usageCode) const { if (usageCode) { - ssize_t index = mKeysByUsageCode.indexOfKey(usageCode); - if (index >= 0) { - return &mKeysByUsageCode.valueAt(index); + auto it = mKeysByUsageCode.find(usageCode); + if (it != mKeysByUsageCode.end()) { + return &it->second; } } if (scanCode) { - ssize_t index = mKeysByScanCode.indexOfKey(scanCode); - if (index >= 0) { - return &mKeysByScanCode.valueAt(index); + auto it = mKeysByScanCode.find(scanCode); + if (it != mKeysByScanCode.end()) { + return &it->second; } } return nullptr; } -status_t KeyLayoutMap::findScanCodesForKey( - int32_t keyCode, std::vector* outScanCodes) const { - const size_t N = mKeysByScanCode.size(); - for (size_t i=0; ipush_back(mKeysByScanCode.keyAt(i)); +std::vector KeyLayoutMap::findScanCodesForKey(int32_t keyCode) const { + std::vector scanCodes; + for (const auto& [scanCode, key] : mKeysByScanCode) { + if (keyCode == key.keyCode) { + scanCodes.push_back(scanCode); } } - return NO_ERROR; + return scanCodes; } -status_t KeyLayoutMap::mapAxis(int32_t scanCode, AxisInfo* outAxisInfo) const { - ssize_t index = mAxes.indexOfKey(scanCode); - if (index < 0) { +std::optional KeyLayoutMap::mapAxis(int32_t scanCode) const { + auto it = mAxes.find(scanCode); + if (it == mAxes.end()) { ALOGD_IF(DEBUG_MAPPING, "mapAxis: scanCode=%d ~ Failed.", scanCode); - return NAME_NOT_FOUND; + return std::nullopt; } - *outAxisInfo = mAxes.valueAt(index); - + const AxisInfo& axisInfo = it->second; ALOGD_IF(DEBUG_MAPPING, "mapAxis: scanCode=%d ~ Result mode=%d, axis=%d, highAxis=%d, " "splitValue=%d, flatOverride=%d.", - scanCode, outAxisInfo->mode, outAxisInfo->axis, outAxisInfo->highAxis, - outAxisInfo->splitValue, outAxisInfo->flatOverride); - - return NO_ERROR; + scanCode, axisInfo.mode, axisInfo.axis, axisInfo.highAxis, axisInfo.splitValue, + axisInfo.flatOverride); + return axisInfo; } -status_t KeyLayoutMap::findScanCodeForLed(int32_t ledCode, int32_t* outScanCode) const { - const size_t N = mLedsByScanCode.size(); - for (size_t i = 0; i < N; i++) { - if (mLedsByScanCode.valueAt(i).ledCode == ledCode) { - *outScanCode = mLedsByScanCode.keyAt(i); - ALOGD_IF(DEBUG_MAPPING, "findScanCodeForLed: ledCode=%d, scanCode=%d.", ledCode, - *outScanCode); - return NO_ERROR; +std::optional KeyLayoutMap::findScanCodeForLed(int32_t ledCode) const { + for (const auto& [scanCode, led] : mLedsByScanCode) { + if (led.ledCode == ledCode) { + ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d, scanCode=%d.", __func__, ledCode, scanCode); + return scanCode; } } - ALOGD_IF(DEBUG_MAPPING, "findScanCodeForLed: ledCode=%d ~ Not found.", ledCode); - return NAME_NOT_FOUND; + ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d ~ Not found.", __func__, ledCode); + return std::nullopt; } -status_t KeyLayoutMap::findUsageCodeForLed(int32_t ledCode, int32_t* outUsageCode) const { - const size_t N = mLedsByUsageCode.size(); - for (size_t i = 0; i < N; i++) { - if (mLedsByUsageCode.valueAt(i).ledCode == ledCode) { - *outUsageCode = mLedsByUsageCode.keyAt(i); - ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d, usage=%x.", __func__, ledCode, *outUsageCode); - return NO_ERROR; +std::optional KeyLayoutMap::findUsageCodeForLed(int32_t ledCode) const { + for (const auto& [usageCode, led] : mLedsByUsageCode) { + if (led.ledCode == ledCode) { + ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d, usage=%x.", __func__, ledCode, usageCode); + return usageCode; } } ALOGD_IF(DEBUG_MAPPING, "%s: ledCode=%d ~ Not found.", __func__, ledCode); - - return NAME_NOT_FOUND; + return std::nullopt; } - // --- KeyLayoutMap::Parser --- KeyLayoutMap::Parser::Parser(KeyLayoutMap* map, Tokenizer* tokenizer) : @@ -314,8 +304,9 @@ status_t KeyLayoutMap::Parser::parseKey() { mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; } - KeyedVector& map = mapUsage ? mMap->mKeysByUsageCode : mMap->mKeysByScanCode; - if (map.indexOfKey(code) >= 0) { + std::unordered_map& map = + mapUsage ? mMap->mKeysByUsageCode : mMap->mKeysByScanCode; + if (map.find(code) != map.end()) { ALOGE("%s: Duplicate entry for key %s '%s'.", mTokenizer->getLocation().string(), mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; @@ -356,7 +347,7 @@ status_t KeyLayoutMap::Parser::parseKey() { Key key; key.keyCode = keyCode; key.flags = flags; - map.add(code, key); + map.insert({code, key}); return NO_ERROR; } @@ -369,7 +360,7 @@ status_t KeyLayoutMap::Parser::parseAxis() { scanCodeToken.string()); return BAD_VALUE; } - if (mMap->mAxes.indexOfKey(scanCode) >= 0) { + if (mMap->mAxes.find(scanCode) != mMap->mAxes.end()) { ALOGE("%s: Duplicate entry for axis scan code '%s'.", mTokenizer->getLocation().string(), scanCodeToken.string()); return BAD_VALUE; @@ -455,8 +446,7 @@ status_t KeyLayoutMap::Parser::parseAxis() { "splitValue=%d, flatOverride=%d.", scanCode, axisInfo.mode, axisInfo.axis, axisInfo.highAxis, axisInfo.splitValue, axisInfo.flatOverride); - - mMap->mAxes.add(scanCode, axisInfo); + mMap->mAxes.insert({scanCode, axisInfo}); return NO_ERROR; } @@ -476,8 +466,9 @@ status_t KeyLayoutMap::Parser::parseLed() { return BAD_VALUE; } - KeyedVector& map = mapUsage ? mMap->mLedsByUsageCode : mMap->mLedsByScanCode; - if (map.indexOfKey(code) >= 0) { + std::unordered_map& map = + mapUsage ? mMap->mLedsByUsageCode : mMap->mLedsByScanCode; + if (map.find(code) != map.end()) { ALOGE("%s: Duplicate entry for led %s '%s'.", mTokenizer->getLocation().string(), mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; @@ -497,7 +488,7 @@ status_t KeyLayoutMap::Parser::parseLed() { Led led; led.ledCode = ledCode; - map.add(code, led); + map.insert({code, led}); return NO_ERROR; } diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index d6a6bd214e..669d2e1833 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -452,8 +452,7 @@ bool EventHub::Device::hasKeycodeLocked(int keycode) const { return false; } - std::vector scanCodes; - keyMap.keyLayoutMap->findScanCodesForKey(keycode, &scanCodes); + std::vector scanCodes = keyMap.keyLayoutMap->findScanCodesForKey(keycode); const size_t N = scanCodes.size(); for (size_t i = 0; i < N && i <= KEY_MAX; i++) { int32_t sc = scanCodes[i]; @@ -548,10 +547,10 @@ status_t EventHub::Device::mapLed(int32_t led, int32_t* outScanCode) const { return NAME_NOT_FOUND; } - int32_t scanCode; - if (keyMap.keyLayoutMap->findScanCodeForLed(led, &scanCode) != NAME_NOT_FOUND) { - if (scanCode >= 0 && scanCode <= LED_MAX && ledBitmask.test(scanCode)) { - *outScanCode = scanCode; + std::optional scanCode = keyMap.keyLayoutMap->findScanCodeForLed(led); + if (scanCode.has_value()) { + if (*scanCode >= 0 && *scanCode <= LED_MAX && ledBitmask.test(*scanCode)) { + *outScanCode = *scanCode; return NO_ERROR; } } @@ -865,8 +864,7 @@ int32_t EventHub::getKeyCodeState(int32_t deviceId, int32_t keyCode) const { Device* device = getDeviceLocked(deviceId); if (device != nullptr && device->hasValidFd() && device->keyMap.haveKeyLayout()) { - std::vector scanCodes; - device->keyMap.keyLayoutMap->findScanCodesForKey(keyCode, &scanCodes); + std::vector scanCodes = device->keyMap.keyLayoutMap->findScanCodesForKey(keyCode); if (scanCodes.size() != 0) { if (device->readDeviceBitMask(EVIOCGKEY(0), device->keyState) >= 0) { for (size_t i = 0; i < scanCodes.size(); i++) { @@ -890,8 +888,8 @@ int32_t EventHub::getKeyCodeForKeyLocation(int32_t deviceId, int32_t locationKey device->keyMap.keyLayoutMap == nullptr) { return AKEYCODE_UNKNOWN; } - std::vector scanCodes; - device->keyMap.keyLayoutMap->findScanCodesForKey(locationKeyCode, &scanCodes); + std::vector scanCodes = + device->keyMap.keyLayoutMap->findScanCodesForKey(locationKeyCode); if (scanCodes.empty()) { ALOGW("Failed to get key code for key location: no scan code maps to key code %d for input" "device %d", @@ -960,20 +958,16 @@ bool EventHub::markSupportedKeyCodes(int32_t deviceId, size_t numCodes, const in Device* device = getDeviceLocked(deviceId); if (device != nullptr && device->keyMap.haveKeyLayout()) { - std::vector scanCodes; for (size_t codeIndex = 0; codeIndex < numCodes; codeIndex++) { - scanCodes.clear(); - - status_t err = device->keyMap.keyLayoutMap->findScanCodesForKey(keyCodes[codeIndex], - &scanCodes); - if (!err) { - // check the possible scan codes identified by the layout map against the - // map of codes actually emitted by the driver - for (size_t sc = 0; sc < scanCodes.size(); sc++) { - if (device->keyBitmask.test(scanCodes[sc])) { - outFlags[codeIndex] = 1; - break; - } + std::vector scanCodes = + device->keyMap.keyLayoutMap->findScanCodesForKey(keyCodes[codeIndex]); + + // check the possible scan codes identified by the layout map against the + // map of codes actually emitted by the driver + for (size_t sc = 0; sc < scanCodes.size(); sc++) { + if (device->keyBitmask.test(scanCodes[sc])) { + outFlags[codeIndex] = 1; + break; } } } @@ -1027,14 +1021,15 @@ status_t EventHub::mapAxis(int32_t deviceId, int32_t scanCode, AxisInfo* outAxis std::scoped_lock _l(mLock); Device* device = getDeviceLocked(deviceId); - if (device != nullptr && device->keyMap.haveKeyLayout()) { - status_t err = device->keyMap.keyLayoutMap->mapAxis(scanCode, outAxisInfo); - if (err == NO_ERROR) { - return NO_ERROR; - } + if (device == nullptr || !device->keyMap.haveKeyLayout()) { + return NAME_NOT_FOUND; } - - return NAME_NOT_FOUND; + std::optional info = device->keyMap.keyLayoutMap->mapAxis(scanCode); + if (!info.has_value()) { + return NAME_NOT_FOUND; + } + *outAxisInfo = *info; + return NO_ERROR; } base::Result> EventHub::mapSensor(int32_t deviceId, -- cgit v1.2.3-59-g8ed1b From a9fd82c447a39ad44a6a4cb908147424e280ba39 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 18 May 2022 09:42:52 -0700 Subject: Do not load keylayout if required kernel module is missing Some key layouts require the presence of a specific kernel module. If the kernel module is not present, the layout should not be loaded. In this CL, we add an option to specify which kernel modules / configs are needed inside the kl file. Bug: 228005926 Test: atest libinput_tests Change-Id: I0d2ab6298bd41df6dc56120bf0385e10da6c3bfe --- include/input/InputDevice.h | 6 +- include/input/KeyLayoutMap.h | 6 +- include/input/Keyboard.h | 4 +- libs/input/Android.bp | 1 + libs/input/InputDevice.cpp | 30 ++++---- libs/input/KeyLayoutMap.cpp | 82 +++++++++++++++++----- libs/input/Keyboard.cpp | 39 ++++++---- libs/input/tests/Android.bp | 3 +- libs/input/tests/InputDevice_test.cpp | 16 +++++ .../tests/data/kl_with_required_fake_config.kl | 20 ++++++ .../tests/data/kl_with_required_real_config.kl | 21 ++++++ 11 files changed, 180 insertions(+), 48 deletions(-) create mode 100644 libs/input/tests/data/kl_with_required_fake_config.kl create mode 100644 libs/input/tests/data/kl_with_required_real_config.kl (limited to 'libs/input/KeyLayoutMap.cpp') diff --git a/include/input/InputDevice.h b/include/input/InputDevice.h index c4f03c9119..3585392c2b 100644 --- a/include/input/InputDevice.h +++ b/include/input/InputDevice.h @@ -300,6 +300,8 @@ enum class InputDeviceConfigurationFileType : int32_t { /* * Gets the path of an input device configuration file, if one is available. * Considers both system provided and user installed configuration files. + * The optional suffix is appended to the end of the file name (before the + * extension). * * The device identifier is used to construct several default configuration file * names to try based on the device name, vendor, product, and version. @@ -307,8 +309,8 @@ enum class InputDeviceConfigurationFileType : int32_t { * Returns an empty string if not found. */ extern std::string getInputDeviceConfigurationFilePathByDeviceIdentifier( - const InputDeviceIdentifier& deviceIdentifier, - InputDeviceConfigurationFileType type); + const InputDeviceIdentifier& deviceIdentifier, InputDeviceConfigurationFileType type, + const char* suffix = ""); /* * Gets the path of an input device configuration file, if one is available. diff --git a/include/input/KeyLayoutMap.h b/include/input/KeyLayoutMap.h index 006c0684e3..1da78aa0c1 100644 --- a/include/input/KeyLayoutMap.h +++ b/include/input/KeyLayoutMap.h @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -63,7 +64,8 @@ struct AxisInfo { */ class KeyLayoutMap { public: - static base::Result> load(const std::string& filename); + static base::Result> load(const std::string& filename, + const char* contents = nullptr); static base::Result> loadContents(const std::string& filename, const char* contents); @@ -103,6 +105,7 @@ private: std::unordered_map mLedsByScanCode; std::unordered_map mLedsByUsageCode; std::unordered_map mSensorsByAbsCode; + std::set mRequiredKernelConfigs; std::string mLoadFileName; KeyLayoutMap(); @@ -123,6 +126,7 @@ private: status_t parseAxis(); status_t parseLed(); status_t parseSensor(); + status_t parseRequiredKernelConfig(); }; }; diff --git a/include/input/Keyboard.h b/include/input/Keyboard.h index 08ad8c6e5a..9a3e15f1cd 100644 --- a/include/input/Keyboard.h +++ b/include/input/Keyboard.h @@ -61,9 +61,7 @@ private: bool probeKeyMap(const InputDeviceIdentifier& deviceIdentifier, const std::string& name); status_t loadKeyLayout(const InputDeviceIdentifier& deviceIdentifier, const std::string& name); status_t loadKeyCharacterMap(const InputDeviceIdentifier& deviceIdentifier, - const std::string& name); - std::string getPath(const InputDeviceIdentifier& deviceIdentifier, - const std::string& name, InputDeviceConfigurationFileType type); + const std::string& name); }; /** diff --git a/libs/input/Android.bp b/libs/input/Android.bp index 3b91ef250d..5030d60da2 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -64,6 +64,7 @@ cc_library { "libbase", "liblog", "libcutils", + "libvintf", ], static_libs: [ diff --git a/libs/input/InputDevice.cpp b/libs/input/InputDevice.cpp index 0bee1b6f2a..a9089690b0 100644 --- a/libs/input/InputDevice.cpp +++ b/libs/input/InputDevice.cpp @@ -53,33 +53,39 @@ static void appendInputDeviceConfigurationFileRelativePath(std::string& path, } std::string getInputDeviceConfigurationFilePathByDeviceIdentifier( - const InputDeviceIdentifier& deviceIdentifier, - InputDeviceConfigurationFileType type) { + const InputDeviceIdentifier& deviceIdentifier, InputDeviceConfigurationFileType type, + const char* suffix) { if (deviceIdentifier.vendor !=0 && deviceIdentifier.product != 0) { if (deviceIdentifier.version != 0) { // Try vendor product version. - std::string versionPath = getInputDeviceConfigurationFilePathByName( - StringPrintf("Vendor_%04x_Product_%04x_Version_%04x", - deviceIdentifier.vendor, deviceIdentifier.product, - deviceIdentifier.version), - type); + std::string versionPath = + getInputDeviceConfigurationFilePathByName(StringPrintf("Vendor_%04x_Product_%" + "04x_Version_%04x%s", + deviceIdentifier.vendor, + deviceIdentifier.product, + deviceIdentifier.version, + suffix), + type); if (!versionPath.empty()) { return versionPath; } } // Try vendor product. - std::string productPath = getInputDeviceConfigurationFilePathByName( - StringPrintf("Vendor_%04x_Product_%04x", - deviceIdentifier.vendor, deviceIdentifier.product), - type); + std::string productPath = + getInputDeviceConfigurationFilePathByName(StringPrintf("Vendor_%04x_Product_%04x%s", + deviceIdentifier.vendor, + deviceIdentifier.product, + suffix), + type); if (!productPath.empty()) { return productPath; } } // Try device name. - return getInputDeviceConfigurationFilePathByName(deviceIdentifier.getCanonicalName(), type); + return getInputDeviceConfigurationFilePathByName(deviceIdentifier.getCanonicalName() + suffix, + type); } std::string getInputDeviceConfigurationFilePathByName( diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index 00fc051cea..59cc7d1dcd 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include #include @@ -76,6 +78,29 @@ static const std::unordered_map SENSOR_ sensorPair(), sensorPair()}; +bool kernelConfigsArePresent(const std::set& configs) { + std::shared_ptr runtimeInfo = + android::vintf::VintfObject::GetInstance()->getRuntimeInfo( + vintf::RuntimeInfo::FetchFlag::CONFIG_GZ); + LOG_ALWAYS_FATAL_IF(runtimeInfo == nullptr, "Kernel configs could not be fetched"); + + const std::map& kernelConfigs = runtimeInfo->kernelConfigs(); + for (const std::string& requiredConfig : configs) { + const auto configIt = kernelConfigs.find(requiredConfig); + if (configIt == kernelConfigs.end()) { + ALOGI("Required kernel config %s is not found", requiredConfig.c_str()); + return false; + } + const std::string& option = configIt->second; + if (option != "y" && option != "m") { + ALOGI("Required kernel config %s has option %s", requiredConfig.c_str(), + option.c_str()); + return false; + } + } + return true; +} + } // namespace KeyLayoutMap::KeyLayoutMap() = default; @@ -83,32 +108,34 @@ KeyLayoutMap::~KeyLayoutMap() = default; base::Result> KeyLayoutMap::loadContents(const std::string& filename, const char* contents) { - Tokenizer* tokenizer; - status_t status = Tokenizer::fromContents(String8(filename.c_str()), contents, &tokenizer); - if (status) { - ALOGE("Error %d opening key layout map.", status); - return Errorf("Error {} opening key layout map file {}.", status, filename.c_str()); - } - std::unique_ptr t(tokenizer); - auto ret = load(t.get()); - if (ret.ok()) { - (*ret)->mLoadFileName = filename; - } - return ret; + return load(filename, contents); } -base::Result> KeyLayoutMap::load(const std::string& filename) { +base::Result> KeyLayoutMap::load(const std::string& filename, + const char* contents) { Tokenizer* tokenizer; - status_t status = Tokenizer::open(String8(filename.c_str()), &tokenizer); + status_t status; + if (contents == nullptr) { + status = Tokenizer::open(String8(filename.c_str()), &tokenizer); + } else { + status = Tokenizer::fromContents(String8(filename.c_str()), contents, &tokenizer); + } if (status) { ALOGE("Error %d opening key layout map file %s.", status, filename.c_str()); return Errorf("Error {} opening key layout map file {}.", status, filename.c_str()); } std::unique_ptr t(tokenizer); auto ret = load(t.get()); - if (ret.ok()) { - (*ret)->mLoadFileName = filename; + if (!ret.ok()) { + return ret; + } + const std::shared_ptr& map = *ret; + LOG_ALWAYS_FATAL_IF(map == nullptr, "Returned map should not be null if there's no error"); + if (!kernelConfigsArePresent(map->mRequiredKernelConfigs)) { + ALOGI("Not loading %s because the required kernel configs are not set", filename.c_str()); + return Errorf("Missing kernel config"); } + map->mLoadFileName = filename; return ret; } @@ -268,6 +295,10 @@ status_t KeyLayoutMap::Parser::parse() { mTokenizer->skipDelimiters(WHITESPACE); status_t status = parseSensor(); if (status) return status; + } else if (keywordToken == "requires_kernel_config") { + mTokenizer->skipDelimiters(WHITESPACE); + status_t status = parseRequiredKernelConfig(); + if (status) return status; } else { ALOGE("%s: Expected keyword, got '%s'.", mTokenizer->getLocation().string(), keywordToken.string()); @@ -570,4 +601,23 @@ status_t KeyLayoutMap::Parser::parseSensor() { return NO_ERROR; } +// Parse the name of a required kernel config. +// The layout won't be used if the specified kernel config is not present +// Examples: +// requires_kernel_config CONFIG_HID_PLAYSTATION +status_t KeyLayoutMap::Parser::parseRequiredKernelConfig() { + String8 codeToken = mTokenizer->nextToken(WHITESPACE); + std::string configName = codeToken.string(); + + const auto result = mMap->mRequiredKernelConfigs.emplace(configName); + if (!result.second) { + ALOGE("%s: Duplicate entry for required kernel config %s.", + mTokenizer->getLocation().string(), configName.c_str()); + return BAD_VALUE; + } + + ALOGD_IF(DEBUG_PARSER, "Parsed required kernel config: name=%s", configName.c_str()); + return NO_ERROR; +} + } // namespace android diff --git a/libs/input/Keyboard.cpp b/libs/input/Keyboard.cpp index f0895b32ef..c3f5151fd1 100644 --- a/libs/input/Keyboard.cpp +++ b/libs/input/Keyboard.cpp @@ -20,16 +20,23 @@ #include #include -#include +#include #include -#include #include -#include +#include +#include +#include #include -#include namespace android { +static std::string getPath(const InputDeviceIdentifier& deviceIdentifier, const std::string& name, + InputDeviceConfigurationFileType type) { + return name.empty() + ? getInputDeviceConfigurationFilePathByDeviceIdentifier(deviceIdentifier, type) + : getInputDeviceConfigurationFilePathByName(name, type); +} + // --- KeyMap --- KeyMap::KeyMap() { @@ -111,11 +118,25 @@ status_t KeyMap::loadKeyLayout(const InputDeviceIdentifier& deviceIdentifier, } base::Result> ret = KeyLayoutMap::load(path); + if (ret.ok()) { + keyLayoutMap = *ret; + keyLayoutFile = path; + return OK; + } + + // Try to load fallback layout if the regular layout could not be loaded due to missing + // kernel modules + std::string fallbackPath( + getInputDeviceConfigurationFilePathByDeviceIdentifier(deviceIdentifier, + InputDeviceConfigurationFileType:: + KEY_LAYOUT, + "_fallback")); + ret = KeyLayoutMap::load(fallbackPath); if (!ret.ok()) { return ret.error().code(); } keyLayoutMap = *ret; - keyLayoutFile = path; + keyLayoutFile = fallbackPath; return OK; } @@ -137,14 +158,6 @@ status_t KeyMap::loadKeyCharacterMap(const InputDeviceIdentifier& deviceIdentifi return OK; } -std::string KeyMap::getPath(const InputDeviceIdentifier& deviceIdentifier, - const std::string& name, InputDeviceConfigurationFileType type) { - return name.empty() - ? getInputDeviceConfigurationFilePathByDeviceIdentifier(deviceIdentifier, type) - : getInputDeviceConfigurationFilePathByName(name, type); -} - - // --- Global functions --- bool isKeyboardSpecialFunction(const PropertyMap* config) { diff --git a/libs/input/tests/Android.bp b/libs/input/tests/Android.bp index 54f7586d0a..c53811a92d 100644 --- a/libs/input/tests/Android.bp +++ b/libs/input/tests/Android.bp @@ -37,8 +37,9 @@ cc_test { "liblog", "libui", "libutils", + "libvintf", ], - data: ["data/*.kcm"], + data: ["data/*"], test_suites: ["device-tests"], } diff --git a/libs/input/tests/InputDevice_test.cpp b/libs/input/tests/InputDevice_test.cpp index 6b695c4581..e872fa442b 100644 --- a/libs/input/tests/InputDevice_test.cpp +++ b/libs/input/tests/InputDevice_test.cpp @@ -130,4 +130,20 @@ TEST_F(InputDeviceKeyMapTest, keyCharacteMapApplyMultipleOverlaysTest) { ASSERT_EQ(*mKeyMap.keyCharacterMap, *frenchOverlaidKeyCharacterMap); } +TEST(InputDeviceKeyLayoutTest, DoesNotLoadWhenRequiredKernelConfigIsMissing) { + std::string klPath = base::GetExecutableDirectory() + "/data/kl_with_required_fake_config.kl"; + base::Result> ret = KeyLayoutMap::load(klPath); + ASSERT_FALSE(ret.ok()) << "Should not be able to load KeyLayout at " << klPath; + // We assert error message here because it's used by 'validatekeymaps' tool + ASSERT_EQ("Missing kernel config", ret.error().message()); +} + +TEST(InputDeviceKeyLayoutTest, LoadsWhenRequiredKernelConfigIsPresent) { + std::string klPath = base::GetExecutableDirectory() + "/data/kl_with_required_real_config.kl"; + base::Result> ret = KeyLayoutMap::load(klPath); + ASSERT_TRUE(ret.ok()) << "Cannot load KeyLayout at " << klPath; + const std::shared_ptr& map = *ret; + ASSERT_NE(nullptr, map) << "Map should be valid because CONFIG_UHID should always be present"; +} + } // namespace android diff --git a/libs/input/tests/data/kl_with_required_fake_config.kl b/libs/input/tests/data/kl_with_required_fake_config.kl new file mode 100644 index 0000000000..2d0a507fbd --- /dev/null +++ b/libs/input/tests/data/kl_with_required_fake_config.kl @@ -0,0 +1,20 @@ +# Copyright (C) 2022 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This KL should not be loaded unless the below config is present in the kernel +# This config will never exist, and therefore this KL should never be loaded +requires_kernel_config CONFIG_HID_FAKEMODULE + +# An arbitrary mapping taken from another file +key 0x130 BUTTON_X \ No newline at end of file diff --git a/libs/input/tests/data/kl_with_required_real_config.kl b/libs/input/tests/data/kl_with_required_real_config.kl new file mode 100644 index 0000000000..303b23e48a --- /dev/null +++ b/libs/input/tests/data/kl_with_required_real_config.kl @@ -0,0 +1,21 @@ +# Copyright (C) 2022 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This KL should not be loaded unless the below config is present in the kernel +# The CONFIG_UHID option has been required for a while, and therefore it's safe +# to assume that this will always be loaded +requires_kernel_config CONFIG_UHID + +# An arbitrary mapping taken from another file +key 0x130 BUTTON_X \ No newline at end of file -- cgit v1.2.3-59-g8ed1b From 8fc145fa0c9f677db178e1ab7bb19aa926367816 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 17 Aug 2022 16:07:40 -0700 Subject: Do not read kernel configs on the host When the code runs on host (so that it can validate the key layout maps), it currently will attempt to read the kernel configs on the host. What we actually want is to either read the kernel configs on the device, or to always assume that the config exists. In this CL, we simply assume that the config exists. Since we are calling this tool for all key layouts, it should not affect the validation. Test: m out/target/common/obj/ETC/validate_framework_keymaps_intermediates/stamp Bug: 237835584 Change-Id: I79158a7591fa4d8ff0a33dcae9df2b138ae33485 --- libs/input/KeyLayoutMap.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'libs/input/KeyLayoutMap.cpp') diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index 59cc7d1dcd..d6b4579a94 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -25,8 +25,10 @@ #include #include #include +#if defined(__ANDROID__) #include #include +#endif #include #include @@ -79,6 +81,7 @@ static const std::unordered_map SENSOR_ sensorPair()}; bool kernelConfigsArePresent(const std::set& configs) { +#if defined(__ANDROID__) std::shared_ptr runtimeInfo = android::vintf::VintfObject::GetInstance()->getRuntimeInfo( vintf::RuntimeInfo::FetchFlag::CONFIG_GZ); @@ -99,6 +102,10 @@ bool kernelConfigsArePresent(const std::set& configs) { } } return true; +#else + (void)configs; // Suppress 'unused variable' warning + return true; +#endif } } // namespace -- cgit v1.2.3-59-g8ed1b From ae4ff289d1ac2ac4f5a5b5bac9c4eb3b837a464b Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Tue, 23 Aug 2022 16:21:39 +0000 Subject: Improve EventHub const-correctness Ensure that all methods that "get" state from event hub are const-correct. While some "set" methods can also be const in event hub because they write to the system (side effect) instead of changing state, we leave these as non-const to make it easier for testing. Bug: None Test: atest inputflinger_tests Change-Id: I086b50458203a2395b1960e2bc1102610a3c0801 --- include/input/KeyLayoutMap.h | 2 +- libs/input/KeyLayoutMap.cpp | 3 +- services/inputflinger/reader/EventHub.cpp | 25 ++++++------ services/inputflinger/reader/include/EventHub.h | 49 +++++++++++++----------- services/inputflinger/tests/InputReader_test.cpp | 33 ++++++++-------- 5 files changed, 57 insertions(+), 55 deletions(-) (limited to 'libs/input/KeyLayoutMap.cpp') diff --git a/include/input/KeyLayoutMap.h b/include/input/KeyLayoutMap.h index 1da78aa0c1..a6c696df26 100644 --- a/include/input/KeyLayoutMap.h +++ b/include/input/KeyLayoutMap.h @@ -78,7 +78,7 @@ public: std::optional mapAxis(int32_t scanCode) const; const std::string getLoadFileName() const; // Return pair of sensor type and sensor data index, for the input device abs code - base::Result> mapSensor(int32_t absCode); + base::Result> mapSensor(int32_t absCode) const; virtual ~KeyLayoutMap(); diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index 59cc7d1dcd..31aed517dd 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -185,7 +185,8 @@ status_t KeyLayoutMap::mapKey(int32_t scanCode, int32_t usageCode, } // Return pair of sensor type and sensor data index, for the input device abs code -base::Result> KeyLayoutMap::mapSensor(int32_t absCode) { +base::Result> KeyLayoutMap::mapSensor( + int32_t absCode) const { auto it = mSensorsByAbsCode.find(absCode); if (it == mSensorsByAbsCode.end()) { ALOGD_IF(DEBUG_MAPPING, "mapSensor: absCode=%d, ~ Failed.", absCode); diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index 5744b83d64..06a38c8f0f 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -1042,7 +1042,7 @@ status_t EventHub::mapAxis(int32_t deviceId, int32_t scanCode, AxisInfo* outAxis } base::Result> EventHub::mapSensor(int32_t deviceId, - int32_t absCode) { + int32_t absCode) const { std::scoped_lock _l(mLock); Device* device = getDeviceLocked(deviceId); @@ -1064,7 +1064,7 @@ const std::unordered_map& EventHub::getBatteryInfoLocke return device->associatedDevice->batteryInfos; } -const std::vector EventHub::getRawBatteryIds(int32_t deviceId) { +std::vector EventHub::getRawBatteryIds(int32_t deviceId) const { std::scoped_lock _l(mLock); std::vector batteryIds; @@ -1075,7 +1075,8 @@ const std::vector EventHub::getRawBatteryIds(int32_t deviceId) { return batteryIds; } -std::optional EventHub::getRawBatteryInfo(int32_t deviceId, int32_t batteryId) { +std::optional EventHub::getRawBatteryInfo(int32_t deviceId, + int32_t batteryId) const { std::scoped_lock _l(mLock); const auto infos = getBatteryInfoLocked(deviceId); @@ -1100,7 +1101,7 @@ const std::unordered_map& EventHub::getLightInfoLocked( return device->associatedDevice->lightInfos; } -const std::vector EventHub::getRawLightIds(int32_t deviceId) { +std::vector EventHub::getRawLightIds(int32_t deviceId) const { std::scoped_lock _l(mLock); std::vector lightIds; @@ -1111,7 +1112,7 @@ const std::vector EventHub::getRawLightIds(int32_t deviceId) { return lightIds; } -std::optional EventHub::getRawLightInfo(int32_t deviceId, int32_t lightId) { +std::optional EventHub::getRawLightInfo(int32_t deviceId, int32_t lightId) const { std::scoped_lock _l(mLock); const auto infos = getLightInfoLocked(deviceId); @@ -1124,7 +1125,7 @@ std::optional EventHub::getRawLightInfo(int32_t deviceId, int32_t return std::nullopt; } -std::optional EventHub::getLightBrightness(int32_t deviceId, int32_t lightId) { +std::optional EventHub::getLightBrightness(int32_t deviceId, int32_t lightId) const { std::scoped_lock _l(mLock); const auto infos = getLightInfoLocked(deviceId); @@ -1141,7 +1142,7 @@ std::optional EventHub::getLightBrightness(int32_t deviceId, int32_t li } std::optional> EventHub::getLightIntensities( - int32_t deviceId, int32_t lightId) { + int32_t deviceId, int32_t lightId) const { std::scoped_lock _l(mLock); const auto infos = getLightInfoLocked(deviceId); @@ -1444,7 +1445,7 @@ void EventHub::cancelVibrate(int32_t deviceId) { } } -std::vector EventHub::getVibratorIds(int32_t deviceId) { +std::vector EventHub::getVibratorIds(int32_t deviceId) const { std::scoped_lock _l(mLock); std::vector vibrators; Device* device = getDeviceLocked(deviceId); @@ -2295,7 +2296,7 @@ bool EventHub::tryAddVideoDeviceLocked(EventHub::Device& device, return true; } -bool EventHub::isDeviceEnabled(int32_t deviceId) { +bool EventHub::isDeviceEnabled(int32_t deviceId) const { std::scoped_lock _l(mLock); Device* device = getDeviceLocked(deviceId); if (device == nullptr) { @@ -2519,7 +2520,7 @@ void EventHub::requestReopenDevices() { mNeedToReopenDevices = true; } -void EventHub::dump(std::string& dump) { +void EventHub::dump(std::string& dump) const { dump += "Event Hub State:\n"; { // acquire lock @@ -2573,9 +2574,9 @@ void EventHub::dump(std::string& dump) { } // release lock } -void EventHub::monitor() { +void EventHub::monitor() const { // Acquire and release the lock to ensure that the event hub has not deadlocked. std::unique_lock lock(mLock); } -}; // namespace android +} // namespace android diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h index 60460b4105..8aec367795 100644 --- a/services/inputflinger/reader/include/EventHub.h +++ b/services/inputflinger/reader/include/EventHub.h @@ -282,22 +282,23 @@ public: */ virtual size_t getEvents(int timeoutMillis, RawEvent* buffer, size_t bufferSize) = 0; virtual std::vector getVideoFrames(int32_t deviceId) = 0; - virtual base::Result> mapSensor(int32_t deviceId, - int32_t absCode) = 0; + virtual base::Result> mapSensor( + int32_t deviceId, int32_t absCode) const = 0; // Raw batteries are sysfs power_supply nodes we found from the EventHub device sysfs node, // containing the raw info of the sysfs node structure. - virtual const std::vector getRawBatteryIds(int32_t deviceId) = 0; + virtual std::vector getRawBatteryIds(int32_t deviceId) const = 0; virtual std::optional getRawBatteryInfo(int32_t deviceId, - int32_t BatteryId) = 0; + int32_t BatteryId) const = 0; // Raw lights are sysfs led light nodes we found from the EventHub device sysfs node, // containing the raw info of the sysfs node structure. - virtual const std::vector getRawLightIds(int32_t deviceId) = 0; - virtual std::optional getRawLightInfo(int32_t deviceId, int32_t lightId) = 0; - virtual std::optional getLightBrightness(int32_t deviceId, int32_t lightId) = 0; + virtual std::vector getRawLightIds(int32_t deviceId) const = 0; + virtual std::optional getRawLightInfo(int32_t deviceId, + int32_t lightId) const = 0; + virtual std::optional getLightBrightness(int32_t deviceId, int32_t lightId) const = 0; virtual void setLightBrightness(int32_t deviceId, int32_t lightId, int32_t brightness) = 0; virtual std::optional> getLightIntensities( - int32_t deviceId, int32_t lightId) = 0; + int32_t deviceId, int32_t lightId) const = 0; virtual void setLightIntensities(int32_t deviceId, int32_t lightId, std::unordered_map intensities) = 0; /* @@ -333,7 +334,7 @@ public: /* Control the vibrator. */ virtual void vibrate(int32_t deviceId, const VibrationElement& effect) = 0; virtual void cancelVibrate(int32_t deviceId) = 0; - virtual std::vector getVibratorIds(int32_t deviceId) = 0; + virtual std::vector getVibratorIds(int32_t deviceId) const = 0; /* Query battery level. */ virtual std::optional getBatteryCapacity(int32_t deviceId, @@ -349,13 +350,13 @@ public: virtual void wake() = 0; /* Dump EventHub state to a string. */ - virtual void dump(std::string& dump) = 0; + virtual void dump(std::string& dump) const = 0; /* Called by the heatbeat to ensures that the reader has not deadlocked. */ - virtual void monitor() = 0; + virtual void monitor() const = 0; /* Return true if the device is enabled. */ - virtual bool isDeviceEnabled(int32_t deviceId) = 0; + virtual bool isDeviceEnabled(int32_t deviceId) const = 0; /* Enable an input device */ virtual status_t enableDevice(int32_t deviceId) = 0; @@ -463,20 +464,22 @@ public: AxisInfo* outAxisInfo) const override final; base::Result> mapSensor( - int32_t deviceId, int32_t absCode) override final; + int32_t deviceId, int32_t absCode) const override final; - const std::vector getRawBatteryIds(int32_t deviceId) override final; + std::vector getRawBatteryIds(int32_t deviceId) const override final; std::optional getRawBatteryInfo(int32_t deviceId, - int32_t BatteryId) override final; + int32_t BatteryId) const override final; - const std::vector getRawLightIds(int32_t deviceId) override final; + std::vector getRawLightIds(int32_t deviceId) const override final; - std::optional getRawLightInfo(int32_t deviceId, int32_t lightId) override final; + std::optional getRawLightInfo(int32_t deviceId, + int32_t lightId) const override final; - std::optional getLightBrightness(int32_t deviceId, int32_t lightId) override final; + std::optional getLightBrightness(int32_t deviceId, + int32_t lightId) const override final; void setLightBrightness(int32_t deviceId, int32_t lightId, int32_t brightness) override final; std::optional> getLightIntensities( - int32_t deviceId, int32_t lightId) override final; + int32_t deviceId, int32_t lightId) const override final; void setLightIntensities(int32_t deviceId, int32_t lightId, std::unordered_map intensities) override final; @@ -512,15 +515,15 @@ public: void vibrate(int32_t deviceId, const VibrationElement& effect) override final; void cancelVibrate(int32_t deviceId) override final; - std::vector getVibratorIds(int32_t deviceId) override final; + std::vector getVibratorIds(int32_t deviceId) const override final; void requestReopenDevices() override final; void wake() override final; - void dump(std::string& dump) override final; + void dump(std::string& dump) const override final; - void monitor() override final; + void monitor() const override final; std::optional getBatteryCapacity(int32_t deviceId, int32_t batteryId) const override final; @@ -528,7 +531,7 @@ public: std::optional getBatteryStatus(int32_t deviceId, int32_t batteryId) const override final; - bool isDeviceEnabled(int32_t deviceId) override final; + bool isDeviceEnabled(int32_t deviceId) const override final; status_t enableDevice(int32_t deviceId) override final; diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index e80a95648b..f1c1ffcf66 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -509,7 +509,7 @@ public: enqueueEvent(ARBITRARY_TIME, READ_TIME, deviceId, EventHubInterface::DEVICE_REMOVED, 0, 0); } - bool isDeviceEnabled(int32_t deviceId) { + bool isDeviceEnabled(int32_t deviceId) const override { Device* device = getDevice(deviceId); if (device == nullptr) { ALOGE("Incorrect device id=%" PRId32 " provided to %s", deviceId, __func__); @@ -518,7 +518,7 @@ public: return device->enabled; } - status_t enableDevice(int32_t deviceId) { + status_t enableDevice(int32_t deviceId) override { status_t result; Device* device = getDevice(deviceId); if (device == nullptr) { @@ -533,7 +533,7 @@ public: return result; } - status_t disableDevice(int32_t deviceId) { + status_t disableDevice(int32_t deviceId) override { Device* device = getDevice(deviceId); if (device == nullptr) { ALOGE("Incorrect device id=%" PRId32 " provided to %s", deviceId, __func__); @@ -795,8 +795,8 @@ private: status_t mapAxis(int32_t, int32_t, AxisInfo*) const override { return NAME_NOT_FOUND; } - base::Result> mapSensor(int32_t deviceId, - int32_t absCode) { + base::Result> mapSensor( + int32_t deviceId, int32_t absCode) const override { Device* device = getDevice(deviceId); if (!device) { return Errorf("Sensor device not found."); @@ -981,7 +981,7 @@ private: void cancelVibrate(int32_t) override {} - std::vector getVibratorIds(int32_t deviceId) override { return mVibrators; }; + std::vector getVibratorIds(int32_t deviceId) const override { return mVibrators; }; std::optional getBatteryCapacity(int32_t, int32_t) const override { return BATTERY_CAPACITY; @@ -991,13 +991,14 @@ private: return BATTERY_STATUS; } - const std::vector getRawBatteryIds(int32_t deviceId) { return {}; } + std::vector getRawBatteryIds(int32_t deviceId) const override { return {}; } - std::optional getRawBatteryInfo(int32_t deviceId, int32_t batteryId) { + std::optional getRawBatteryInfo(int32_t deviceId, + int32_t batteryId) const override { return std::nullopt; } - const std::vector getRawLightIds(int32_t deviceId) override { + std::vector getRawLightIds(int32_t deviceId) const override { std::vector ids; for (const auto& [rawId, info] : mRawLightInfos) { ids.push_back(rawId); @@ -1005,7 +1006,7 @@ private: return ids; } - std::optional getRawLightInfo(int32_t deviceId, int32_t lightId) override { + std::optional getRawLightInfo(int32_t deviceId, int32_t lightId) const override { auto it = mRawLightInfos.find(lightId); if (it == mRawLightInfos.end()) { return std::nullopt; @@ -1022,7 +1023,7 @@ private: mLightIntensities.emplace(lightId, intensities); }; - std::optional getLightBrightness(int32_t deviceId, int32_t lightId) override { + std::optional getLightBrightness(int32_t deviceId, int32_t lightId) const override { auto lightIt = mLightBrightness.find(lightId); if (lightIt == mLightBrightness.end()) { return std::nullopt; @@ -1031,7 +1032,7 @@ private: } std::optional> getLightIntensities( - int32_t deviceId, int32_t lightId) override { + int32_t deviceId, int32_t lightId) const override { auto lightIt = mLightIntensities.find(lightId); if (lightIt == mLightIntensities.end()) { return std::nullopt; @@ -1039,13 +1040,9 @@ private: return lightIt->second; }; - virtual bool isExternal(int32_t) const { - return false; - } - - void dump(std::string&) override {} + void dump(std::string&) const override {} - void monitor() override {} + void monitor() const override {} void requestReopenDevices() override {} -- cgit v1.2.3-59-g8ed1b From 5df3493d3cf633f8ac7447bc5474a0dfbc1a8359 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Mon, 23 Jan 2023 12:41:01 -0800 Subject: Validate axes and led labels correctly Before this CL, a number of checks for kl file validity were incorrect. Some of the APIs were supposed to return an invalid value, but instead were always returning a valid value, no matter what the input was. Correct these values by switching to std::optional. Bug: 266400536 Test: m libinput_tests && adb sync data && adb shell -t /data/nativetest64/libinput_tests/libinput_tests Change-Id: I4ef45f3249dca4f4f033fb85e9fecbc2ad1f1395 --- include/input/Input.h | 4 +- include/input/InputEventLabels.h | 12 ++-- libs/input/Input.cpp | 4 +- libs/input/InputEventLabels.cpp | 22 +++---- libs/input/KeyCharacterMap.cpp | 24 ++++---- libs/input/KeyLayoutMap.cpp | 106 +++++++++++++++++++------------- libs/input/tests/InputDevice_test.cpp | 14 +++++ libs/input/tests/data/bad_axis_label.kl | 17 +++++ libs/input/tests/data/bad_led_label.kl | 17 +++++ 9 files changed, 142 insertions(+), 78 deletions(-) create mode 100644 libs/input/tests/data/bad_axis_label.kl create mode 100644 libs/input/tests/data/bad_led_label.kl (limited to 'libs/input/KeyLayoutMap.cpp') diff --git a/include/input/Input.h b/include/input/Input.h index 30b0d6a67a..7573282fc1 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -529,7 +529,7 @@ public: inline nsecs_t getEventTime() const { return mEventTime; } static const char* getLabel(int32_t keyCode); - static int32_t getKeyCodeFromLabel(const char* label); + static std::optional getKeyCodeFromLabel(const char* label); void initialize(int32_t id, int32_t deviceId, uint32_t source, int32_t displayId, std::array hmac, int32_t action, int32_t flags, int32_t keyCode, @@ -842,7 +842,7 @@ public: } static const char* getLabel(int32_t axis); - static int32_t getAxisFromLabel(const char* label); + static std::optional getAxisFromLabel(const char* label); static std::string actionToString(int32_t action); diff --git a/include/input/InputEventLabels.h b/include/input/InputEventLabels.h index b4374acdcc..4668fce116 100644 --- a/include/input/InputEventLabels.h +++ b/include/input/InputEventLabels.h @@ -35,22 +35,22 @@ struct InputEventLabel { class InputEventLookup { public: - static int lookupValueByLabel(const std::unordered_map& map, - const char* literal); + static std::optional lookupValueByLabel(const std::unordered_map& map, + const char* literal); static const char* lookupLabelByValue(const std::vector& vec, int value); - static int32_t getKeyCodeByLabel(const char* label); + static std::optional getKeyCodeByLabel(const char* label); static const char* getLabelByKeyCode(int32_t keyCode); - static uint32_t getKeyFlagByLabel(const char* label); + static std::optional getKeyFlagByLabel(const char* label); - static int32_t getAxisByLabel(const char* label); + static std::optional getAxisByLabel(const char* label); static const char* getAxisLabel(int32_t axisId); - static int32_t getLedByLabel(const char* label); + static std::optional getLedByLabel(const char* label); private: static const std::unordered_map KEYCODES; diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index c356c2e5e9..133b260a61 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -299,7 +299,7 @@ const char* KeyEvent::getLabel(int32_t keyCode) { return InputEventLookup::getLabelByKeyCode(keyCode); } -int32_t KeyEvent::getKeyCodeFromLabel(const char* label) { +std::optional KeyEvent::getKeyCodeFromLabel(const char* label) { return InputEventLookup::getKeyCodeByLabel(label); } @@ -891,7 +891,7 @@ const char* MotionEvent::getLabel(int32_t axis) { return InputEventLookup::getAxisLabel(axis); } -int32_t MotionEvent::getAxisFromLabel(const char* label) { +std::optional MotionEvent::getAxisFromLabel(const char* label) { return InputEventLookup::getAxisByLabel(label); } diff --git a/libs/input/InputEventLabels.cpp b/libs/input/InputEventLabels.cpp index 7159e27b13..d97c1bb629 100644 --- a/libs/input/InputEventLabels.cpp +++ b/libs/input/InputEventLabels.cpp @@ -438,11 +438,11 @@ const std::unordered_map InputEventLookup::LEDS = {LEDS_SEQUEN const std::unordered_map InputEventLookup::FLAGS = {FLAGS_SEQUENCE}; -int InputEventLookup::lookupValueByLabel(const std::unordered_map& map, - const char* literal) { +std::optional InputEventLookup::lookupValueByLabel( + const std::unordered_map& map, const char* literal) { std::string str(literal); auto it = map.find(str); - return it != map.end() ? it->second : 0; + return it != map.end() ? std::make_optional(it->second) : std::nullopt; } const char* InputEventLookup::lookupLabelByValue(const std::vector& vec, @@ -453,8 +453,8 @@ const char* InputEventLookup::lookupLabelByValue(const std::vector InputEventLookup::getKeyCodeByLabel(const char* label) { + return lookupValueByLabel(KEYCODES, label); } const char* InputEventLookup::getLabelByKeyCode(int32_t keyCode) { @@ -464,20 +464,20 @@ const char* InputEventLookup::getLabelByKeyCode(int32_t keyCode) { return nullptr; } -uint32_t InputEventLookup::getKeyFlagByLabel(const char* label) { - return uint32_t(lookupValueByLabel(FLAGS, label)); +std::optional InputEventLookup::getKeyFlagByLabel(const char* label) { + return lookupValueByLabel(FLAGS, label); } -int32_t InputEventLookup::getAxisByLabel(const char* label) { - return int32_t(lookupValueByLabel(AXES, label)); +std::optional InputEventLookup::getAxisByLabel(const char* label) { + return lookupValueByLabel(AXES, label); } const char* InputEventLookup::getAxisLabel(int32_t axisId) { return lookupLabelByValue(AXES_NAMES, axisId); } -int32_t InputEventLookup::getLedByLabel(const char* label) { - return int32_t(lookupValueByLabel(LEDS, label)); +std::optional InputEventLookup::getLedByLabel(const char* label) { + return lookupValueByLabel(LEDS, label); } } // namespace android diff --git a/libs/input/KeyCharacterMap.cpp b/libs/input/KeyCharacterMap.cpp index 6bfac40932..737bd15901 100644 --- a/libs/input/KeyCharacterMap.cpp +++ b/libs/input/KeyCharacterMap.cpp @@ -999,7 +999,7 @@ status_t KeyCharacterMap::Parser::parseMapKey() { mTokenizer->skipDelimiters(WHITESPACE); String8 keyCodeToken = mTokenizer->nextToken(WHITESPACE); - int32_t keyCode = InputEventLookup::getKeyCodeByLabel(keyCodeToken.string()); + std::optional keyCode = InputEventLookup::getKeyCodeByLabel(keyCodeToken.string()); if (!keyCode) { ALOGE("%s: Expected key code label, got '%s'.", mTokenizer->getLocation().string(), keyCodeToken.string()); @@ -1010,19 +1010,19 @@ status_t KeyCharacterMap::Parser::parseMapKey() { ALOGD("Parsed map key %s: code=%d, keyCode=%d.", mapUsage ? "usage" : "scan code", code, keyCode); #endif - map.insert_or_assign(code, keyCode); + map.insert_or_assign(code, *keyCode); return NO_ERROR; } status_t KeyCharacterMap::Parser::parseKey() { String8 keyCodeToken = mTokenizer->nextToken(WHITESPACE); - int32_t keyCode = InputEventLookup::getKeyCodeByLabel(keyCodeToken.string()); + std::optional keyCode = InputEventLookup::getKeyCodeByLabel(keyCodeToken.string()); if (!keyCode) { ALOGE("%s: Expected key code label, got '%s'.", mTokenizer->getLocation().string(), keyCodeToken.string()); return BAD_VALUE; } - if (mMap->mKeys.indexOfKey(keyCode) >= 0) { + if (mMap->mKeys.indexOfKey(*keyCode) >= 0) { ALOGE("%s: Duplicate entry for key code '%s'.", mTokenizer->getLocation().string(), keyCodeToken.string()); return BAD_VALUE; @@ -1036,11 +1036,9 @@ status_t KeyCharacterMap::Parser::parseKey() { return BAD_VALUE; } -#if DEBUG_PARSER - ALOGD("Parsed beginning of key: keyCode=%d.", keyCode); -#endif - mKeyCode = keyCode; - mMap->mKeys.add(keyCode, new Key()); + ALOGD_IF(DEBUG_PARSER, "Parsed beginning of key: keyCode=%d.", *keyCode); + mKeyCode = *keyCode; + mMap->mKeys.add(*keyCode, new Key()); mState = STATE_KEY; return NO_ERROR; } @@ -1136,7 +1134,7 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { } else if (token == "fallback") { mTokenizer->skipDelimiters(WHITESPACE); token = mTokenizer->nextToken(WHITESPACE); - int32_t keyCode = InputEventLookup::getKeyCodeByLabel(token.string()); + std::optional keyCode = InputEventLookup::getKeyCodeByLabel(token.string()); if (!keyCode) { ALOGE("%s: Invalid key code label for fallback behavior, got '%s'.", mTokenizer->getLocation().string(), @@ -1148,12 +1146,12 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { mTokenizer->getLocation().string()); return BAD_VALUE; } - behavior.fallbackKeyCode = keyCode; + behavior.fallbackKeyCode = *keyCode; haveFallback = true; } else if (token == "replace") { mTokenizer->skipDelimiters(WHITESPACE); token = mTokenizer->nextToken(WHITESPACE); - int32_t keyCode = InputEventLookup::getKeyCodeByLabel(token.string()); + std::optional keyCode = InputEventLookup::getKeyCodeByLabel(token.string()); if (!keyCode) { ALOGE("%s: Invalid key code label for replace, got '%s'.", mTokenizer->getLocation().string(), @@ -1170,7 +1168,7 @@ status_t KeyCharacterMap::Parser::parseKeyProperty() { mTokenizer->getLocation().string()); return BAD_VALUE; } - behavior.replacementKeyCode = keyCode; + behavior.replacementKeyCode = *keyCode; haveReplacement = true; } else { diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index 73710330d0..a2649f6f11 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -16,6 +16,7 @@ #define LOG_TAG "KeyLayoutMap" +#include #include #include #include @@ -54,6 +55,21 @@ const bool DEBUG_MAPPING = namespace android { namespace { +std::optional parseInt(const char* str) { + char* end; + errno = 0; + const int value = strtol(str, &end, 0); + if (end == str) { + LOG(ERROR) << "Could not parse " << str; + return {}; + } + if (errno == ERANGE) { + LOG(ERROR) << "Out of bounds: " << str; + return {}; + } + return value; +} + constexpr const char* WHITESPACE = " \t\r"; template @@ -336,16 +352,15 @@ status_t KeyLayoutMap::Parser::parseKey() { codeToken = mTokenizer->nextToken(WHITESPACE); } - char* end; - int32_t code = int32_t(strtol(codeToken.string(), &end, 0)); - if (*end) { + std::optional code = parseInt(codeToken.string()); + if (!code) { ALOGE("%s: Expected key %s number, got '%s'.", mTokenizer->getLocation().string(), mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; } std::unordered_map& map = mapUsage ? mMap->mKeysByUsageCode : mMap->mKeysByScanCode; - if (map.find(code) != map.end()) { + if (map.find(*code) != map.end()) { ALOGE("%s: Duplicate entry for key %s '%s'.", mTokenizer->getLocation().string(), mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; @@ -353,7 +368,7 @@ status_t KeyLayoutMap::Parser::parseKey() { mTokenizer->skipDelimiters(WHITESPACE); String8 keyCodeToken = mTokenizer->nextToken(WHITESPACE); - int32_t keyCode = InputEventLookup::getKeyCodeByLabel(keyCodeToken.string()); + std::optional keyCode = InputEventLookup::getKeyCodeByLabel(keyCodeToken.string()); if (!keyCode) { ALOGE("%s: Expected key code label, got '%s'.", mTokenizer->getLocation().string(), keyCodeToken.string()); @@ -366,40 +381,39 @@ status_t KeyLayoutMap::Parser::parseKey() { if (mTokenizer->isEol() || mTokenizer->peekChar() == '#') break; String8 flagToken = mTokenizer->nextToken(WHITESPACE); - uint32_t flag = InputEventLookup::getKeyFlagByLabel(flagToken.string()); + std::optional flag = InputEventLookup::getKeyFlagByLabel(flagToken.string()); if (!flag) { ALOGE("%s: Expected key flag label, got '%s'.", mTokenizer->getLocation().string(), flagToken.string()); return BAD_VALUE; } - if (flags & flag) { + if (flags & *flag) { ALOGE("%s: Duplicate key flag '%s'.", mTokenizer->getLocation().string(), flagToken.string()); return BAD_VALUE; } - flags |= flag; + flags |= *flag; } ALOGD_IF(DEBUG_PARSER, "Parsed key %s: code=%d, keyCode=%d, flags=0x%08x.", - mapUsage ? "usage" : "scan code", code, keyCode, flags); + mapUsage ? "usage" : "scan code", *code, *keyCode, flags); Key key; - key.keyCode = keyCode; + key.keyCode = *keyCode; key.flags = flags; - map.insert({code, key}); + map.insert({*code, key}); return NO_ERROR; } status_t KeyLayoutMap::Parser::parseAxis() { String8 scanCodeToken = mTokenizer->nextToken(WHITESPACE); - char* end; - int32_t scanCode = int32_t(strtol(scanCodeToken.string(), &end, 0)); - if (*end) { + std::optional scanCode = parseInt(scanCodeToken.string()); + if (!scanCode) { ALOGE("%s: Expected axis scan code number, got '%s'.", mTokenizer->getLocation().string(), scanCodeToken.string()); return BAD_VALUE; } - if (mMap->mAxes.find(scanCode) != mMap->mAxes.end()) { + if (mMap->mAxes.find(*scanCode) != mMap->mAxes.end()) { ALOGE("%s: Duplicate entry for axis scan code '%s'.", mTokenizer->getLocation().string(), scanCodeToken.string()); return BAD_VALUE; @@ -414,48 +428,53 @@ status_t KeyLayoutMap::Parser::parseAxis() { mTokenizer->skipDelimiters(WHITESPACE); String8 axisToken = mTokenizer->nextToken(WHITESPACE); - axisInfo.axis = InputEventLookup::getAxisByLabel(axisToken.string()); - if (axisInfo.axis < 0) { + std::optional axis = InputEventLookup::getAxisByLabel(axisToken.string()); + if (!axis) { ALOGE("%s: Expected inverted axis label, got '%s'.", mTokenizer->getLocation().string(), axisToken.string()); return BAD_VALUE; } + axisInfo.axis = *axis; } else if (token == "split") { axisInfo.mode = AxisInfo::MODE_SPLIT; mTokenizer->skipDelimiters(WHITESPACE); String8 splitToken = mTokenizer->nextToken(WHITESPACE); - axisInfo.splitValue = int32_t(strtol(splitToken.string(), &end, 0)); - if (*end) { + std::optional splitValue = parseInt(splitToken.string()); + if (!splitValue) { ALOGE("%s: Expected split value, got '%s'.", mTokenizer->getLocation().string(), splitToken.string()); return BAD_VALUE; } + axisInfo.splitValue = *splitValue; mTokenizer->skipDelimiters(WHITESPACE); String8 lowAxisToken = mTokenizer->nextToken(WHITESPACE); - axisInfo.axis = InputEventLookup::getAxisByLabel(lowAxisToken.string()); - if (axisInfo.axis < 0) { + std::optional axis = InputEventLookup::getAxisByLabel(lowAxisToken.string()); + if (!axis) { ALOGE("%s: Expected low axis label, got '%s'.", mTokenizer->getLocation().string(), lowAxisToken.string()); return BAD_VALUE; } + axisInfo.axis = *axis; mTokenizer->skipDelimiters(WHITESPACE); String8 highAxisToken = mTokenizer->nextToken(WHITESPACE); - axisInfo.highAxis = InputEventLookup::getAxisByLabel(highAxisToken.string()); - if (axisInfo.highAxis < 0) { + std::optional highAxis = InputEventLookup::getAxisByLabel(highAxisToken.string()); + if (!highAxis) { ALOGE("%s: Expected high axis label, got '%s'.", mTokenizer->getLocation().string(), highAxisToken.string()); return BAD_VALUE; } + axisInfo.highAxis = *highAxis; } else { - axisInfo.axis = InputEventLookup::getAxisByLabel(token.string()); - if (axisInfo.axis < 0) { + std::optional axis = InputEventLookup::getAxisByLabel(token.string()); + if (!axis) { ALOGE("%s: Expected axis label, 'split' or 'invert', got '%s'.", mTokenizer->getLocation().string(), token.string()); return BAD_VALUE; } + axisInfo.axis = *axis; } for (;;) { @@ -467,12 +486,13 @@ status_t KeyLayoutMap::Parser::parseAxis() { if (keywordToken == "flat") { mTokenizer->skipDelimiters(WHITESPACE); String8 flatToken = mTokenizer->nextToken(WHITESPACE); - axisInfo.flatOverride = int32_t(strtol(flatToken.string(), &end, 0)); - if (*end) { + std::optional flatOverride = parseInt(flatToken.string()); + if (!flatOverride) { ALOGE("%s: Expected flat value, got '%s'.", mTokenizer->getLocation().string(), flatToken.string()); return BAD_VALUE; } + axisInfo.flatOverride = *flatOverride; } else { ALOGE("%s: Expected keyword 'flat', got '%s'.", mTokenizer->getLocation().string(), keywordToken.string()); @@ -483,9 +503,9 @@ status_t KeyLayoutMap::Parser::parseAxis() { ALOGD_IF(DEBUG_PARSER, "Parsed axis: scanCode=%d, mode=%d, axis=%d, highAxis=%d, " "splitValue=%d, flatOverride=%d.", - scanCode, axisInfo.mode, axisInfo.axis, axisInfo.highAxis, axisInfo.splitValue, + *scanCode, axisInfo.mode, axisInfo.axis, axisInfo.highAxis, axisInfo.splitValue, axisInfo.flatOverride); - mMap->mAxes.insert({scanCode, axisInfo}); + mMap->mAxes.insert({*scanCode, axisInfo}); return NO_ERROR; } @@ -497,9 +517,8 @@ status_t KeyLayoutMap::Parser::parseLed() { mTokenizer->skipDelimiters(WHITESPACE); codeToken = mTokenizer->nextToken(WHITESPACE); } - char* end; - int32_t code = int32_t(strtol(codeToken.string(), &end, 0)); - if (*end) { + std::optional code = parseInt(codeToken.string()); + if (!code) { ALOGE("%s: Expected led %s number, got '%s'.", mTokenizer->getLocation().string(), mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; @@ -507,7 +526,7 @@ status_t KeyLayoutMap::Parser::parseLed() { std::unordered_map& map = mapUsage ? mMap->mLedsByUsageCode : mMap->mLedsByScanCode; - if (map.find(code) != map.end()) { + if (map.find(*code) != map.end()) { ALOGE("%s: Duplicate entry for led %s '%s'.", mTokenizer->getLocation().string(), mapUsage ? "usage" : "scan code", codeToken.string()); return BAD_VALUE; @@ -515,19 +534,19 @@ status_t KeyLayoutMap::Parser::parseLed() { mTokenizer->skipDelimiters(WHITESPACE); String8 ledCodeToken = mTokenizer->nextToken(WHITESPACE); - int32_t ledCode = InputEventLookup::getLedByLabel(ledCodeToken.string()); - if (ledCode < 0) { + std::optional ledCode = InputEventLookup::getLedByLabel(ledCodeToken.string()); + if (!ledCode) { ALOGE("%s: Expected LED code label, got '%s'.", mTokenizer->getLocation().string(), ledCodeToken.string()); return BAD_VALUE; } ALOGD_IF(DEBUG_PARSER, "Parsed led %s: code=%d, ledCode=%d.", mapUsage ? "usage" : "scan code", - code, ledCode); + *code, *ledCode); Led led; - led.ledCode = ledCode; - map.insert({code, led}); + led.ledCode = *ledCode; + map.insert({*code, led}); return NO_ERROR; } @@ -565,16 +584,15 @@ static std::optional getSensorDataIndex(String8 token) { // sensor 0x05 GYROSCOPE Z status_t KeyLayoutMap::Parser::parseSensor() { String8 codeToken = mTokenizer->nextToken(WHITESPACE); - char* end; - int32_t code = int32_t(strtol(codeToken.string(), &end, 0)); - if (*end) { + std::optional code = parseInt(codeToken.string()); + if (!code) { ALOGE("%s: Expected sensor %s number, got '%s'.", mTokenizer->getLocation().string(), "abs code", codeToken.string()); return BAD_VALUE; } std::unordered_map& map = mMap->mSensorsByAbsCode; - if (map.find(code) != map.end()) { + if (map.find(*code) != map.end()) { ALOGE("%s: Duplicate entry for sensor %s '%s'.", mTokenizer->getLocation().string(), "abs code", codeToken.string()); return BAD_VALUE; @@ -599,13 +617,13 @@ status_t KeyLayoutMap::Parser::parseSensor() { } int32_t sensorDataIndex = indexOpt.value(); - ALOGD_IF(DEBUG_PARSER, "Parsed sensor: abs code=%d, sensorType=%s, sensorDataIndex=%d.", code, + ALOGD_IF(DEBUG_PARSER, "Parsed sensor: abs code=%d, sensorType=%s, sensorDataIndex=%d.", *code, ftl::enum_string(sensorType).c_str(), sensorDataIndex); Sensor sensor; sensor.sensorType = sensorType; sensor.sensorDataIndex = sensorDataIndex; - map.emplace(code, sensor); + map.emplace(*code, sensor); return NO_ERROR; } diff --git a/libs/input/tests/InputDevice_test.cpp b/libs/input/tests/InputDevice_test.cpp index 2344463241..ee961f0ffc 100644 --- a/libs/input/tests/InputDevice_test.cpp +++ b/libs/input/tests/InputDevice_test.cpp @@ -133,6 +133,20 @@ TEST_F(InputDeviceKeyMapTest, keyCharacteMapApplyMultipleOverlaysTest) { ASSERT_EQ(*mKeyMap.keyCharacterMap, *frenchOverlaidKeyCharacterMap); } +TEST_F(InputDeviceKeyMapTest, keyCharacteMapBadAxisLabel) { + std::string klPath = base::GetExecutableDirectory() + "/data/bad_axis_label.kl"; + + base::Result> ret = KeyLayoutMap::load(klPath); + ASSERT_FALSE(ret.ok()) << "Should not be able to load KeyLayout at " << klPath; +} + +TEST_F(InputDeviceKeyMapTest, keyCharacteMapBadLedLabel) { + std::string klPath = base::GetExecutableDirectory() + "/data/bad_led_label.kl"; + + base::Result> ret = KeyLayoutMap::load(klPath); + ASSERT_FALSE(ret.ok()) << "Should not be able to load KeyLayout at " << klPath; +} + TEST(InputDeviceKeyLayoutTest, DoesNotLoadWhenRequiredKernelConfigIsMissing) { #if !defined(__ANDROID__) GTEST_SKIP() << "Can't check kernel configs on host"; diff --git a/libs/input/tests/data/bad_axis_label.kl b/libs/input/tests/data/bad_axis_label.kl new file mode 100644 index 0000000000..689738077c --- /dev/null +++ b/libs/input/tests/data/bad_axis_label.kl @@ -0,0 +1,17 @@ +# Copyright (C) 2023 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This KL should not be loaded because the axis label is not valid + +axis 0 DEFINITELY_NOT_AXIS_LABEL diff --git a/libs/input/tests/data/bad_led_label.kl b/libs/input/tests/data/bad_led_label.kl new file mode 100644 index 0000000000..293c0d2af4 --- /dev/null +++ b/libs/input/tests/data/bad_led_label.kl @@ -0,0 +1,17 @@ +# Copyright (C) 2023 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This KL should not be loaded because the led label is invalid + +led 0 ABSOLUTELY_NOT_LED_LABEL -- cgit v1.2.3-59-g8ed1b