From 1329696700421d25b3ea56041ce0cf96312c616c Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Tue, 11 Aug 2020 13:04:26 -0500 Subject: [PATCH] Move PropertyMap from libutils to libinput The input code is the only customer of PropertyMap. For easier maintenance and eventual removal of it, move it to libinput. Currently, the caller is responsible for managing the lifecycle of the returned outMap when calling PropertyMap::load. However, the fact that the function call allocates new memory is not obvious from the function signature. In a separate commit, I will refactor the function to return Result> to make it less errorprone. In this commit, only move the files around to make code reviews easier. Bug: 163171599 Test: atest inputflinger_tests Change-Id: I316084886c3f09a1776fdb449d2f03d0563b66c1 --- libutils/Android.bp | 7 - libutils/PropertyMap.cpp | 214 --------------------------- libutils/PropertyMap_fuzz.cpp | 76 ---------- libutils/include/utils/PropertyMap.h | 106 ------------- 4 files changed, 403 deletions(-) delete mode 100644 libutils/PropertyMap.cpp delete mode 100755 libutils/PropertyMap_fuzz.cpp delete mode 100644 libutils/include/utils/PropertyMap.h diff --git a/libutils/Android.bp b/libutils/Android.bp index 9c012a983..926e3d764 100644 --- a/libutils/Android.bp +++ b/libutils/Android.bp @@ -132,7 +132,6 @@ cc_library { "JenkinsHash.cpp", "NativeHandle.cpp", "Printer.cpp", - "PropertyMap.cpp", "RefBase.cpp", "SharedBuffer.cpp", "StopWatch.cpp", @@ -269,12 +268,6 @@ cc_fuzz { srcs: ["StopWatch_fuzz.cpp"], } -cc_fuzz { - name: "libutils_fuzz_propertymap", - defaults: ["libutils_fuzz_defaults"], - srcs: ["PropertyMap_fuzz.cpp"], -} - cc_fuzz { name: "libutils_fuzz_rwlock", defaults: ["libutils_fuzz_defaults"], diff --git a/libutils/PropertyMap.cpp b/libutils/PropertyMap.cpp deleted file mode 100644 index f00272a0f..000000000 --- a/libutils/PropertyMap.cpp +++ /dev/null @@ -1,214 +0,0 @@ -/* - * Copyright (C) 2008 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. - */ - -#define LOG_TAG "PropertyMap" - -#include - -// Enables debug output for the parser. -#define DEBUG_PARSER 0 - -// Enables debug output for parser performance. -#define DEBUG_PARSER_PERFORMANCE 0 - - -namespace android { - -static const char* WHITESPACE = " \t\r"; -static const char* WHITESPACE_OR_PROPERTY_DELIMITER = " \t\r="; - - -// --- PropertyMap --- - -PropertyMap::PropertyMap() { -} - -PropertyMap::~PropertyMap() { -} - -void PropertyMap::clear() { - mProperties.clear(); -} - -void PropertyMap::addProperty(const String8& key, const String8& value) { - mProperties.add(key, value); -} - -bool PropertyMap::hasProperty(const String8& key) const { - return mProperties.indexOfKey(key) >= 0; -} - -bool PropertyMap::tryGetProperty(const String8& key, String8& outValue) const { - ssize_t index = mProperties.indexOfKey(key); - if (index < 0) { - return false; - } - - outValue = mProperties.valueAt(index); - return true; -} - -bool PropertyMap::tryGetProperty(const String8& key, bool& outValue) const { - int32_t intValue; - if (!tryGetProperty(key, intValue)) { - return false; - } - - outValue = intValue; - return true; -} - -bool PropertyMap::tryGetProperty(const String8& key, int32_t& outValue) const { - String8 stringValue; - if (! tryGetProperty(key, stringValue) || stringValue.length() == 0) { - return false; - } - - char* end; - int value = strtol(stringValue.string(), & end, 10); - if (*end != '\0') { - ALOGW("Property key '%s' has invalid value '%s'. Expected an integer.", - key.string(), stringValue.string()); - return false; - } - outValue = value; - return true; -} - -bool PropertyMap::tryGetProperty(const String8& key, float& outValue) const { - String8 stringValue; - if (! tryGetProperty(key, stringValue) || stringValue.length() == 0) { - return false; - } - - char* end; - float value = strtof(stringValue.string(), & end); - if (*end != '\0') { - ALOGW("Property key '%s' has invalid value '%s'. Expected a float.", - key.string(), stringValue.string()); - return false; - } - outValue = value; - return true; -} - -void PropertyMap::addAll(const PropertyMap* map) { - for (size_t i = 0; i < map->mProperties.size(); i++) { - mProperties.add(map->mProperties.keyAt(i), map->mProperties.valueAt(i)); - } -} - -status_t PropertyMap::load(const String8& filename, PropertyMap** outMap) { - *outMap = nullptr; - - Tokenizer* tokenizer; - status_t status = Tokenizer::open(filename, &tokenizer); - if (status) { - ALOGE("Error %d opening property file %s.", status, filename.string()); - } else { - PropertyMap* map = new PropertyMap(); - if (!map) { - ALOGE("Error allocating property map."); - status = NO_MEMORY; - } else { -#if DEBUG_PARSER_PERFORMANCE - nsecs_t startTime = systemTime(SYSTEM_TIME_MONOTONIC); -#endif - Parser parser(map, tokenizer); - status = parser.parse(); -#if DEBUG_PARSER_PERFORMANCE - nsecs_t elapsedTime = systemTime(SYSTEM_TIME_MONOTONIC) - startTime; - ALOGD("Parsed property file '%s' %d lines in %0.3fms.", - tokenizer->getFilename().string(), tokenizer->getLineNumber(), - elapsedTime / 1000000.0); -#endif - if (status) { - delete map; - } else { - *outMap = map; - } - } - delete tokenizer; - } - return status; -} - - -// --- PropertyMap::Parser --- - -PropertyMap::Parser::Parser(PropertyMap* map, Tokenizer* tokenizer) : - mMap(map), mTokenizer(tokenizer) { -} - -PropertyMap::Parser::~Parser() { -} - -status_t PropertyMap::Parser::parse() { - while (!mTokenizer->isEof()) { -#if DEBUG_PARSER - ALOGD("Parsing %s: '%s'.", mTokenizer->getLocation().string(), - mTokenizer->peekRemainderOfLine().string()); -#endif - - mTokenizer->skipDelimiters(WHITESPACE); - - if (!mTokenizer->isEol() && mTokenizer->peekChar() != '#') { - String8 keyToken = mTokenizer->nextToken(WHITESPACE_OR_PROPERTY_DELIMITER); - if (keyToken.isEmpty()) { - ALOGE("%s: Expected non-empty property key.", mTokenizer->getLocation().string()); - return BAD_VALUE; - } - - mTokenizer->skipDelimiters(WHITESPACE); - - if (mTokenizer->nextChar() != '=') { - ALOGE("%s: Expected '=' between property key and value.", - mTokenizer->getLocation().string()); - return BAD_VALUE; - } - - mTokenizer->skipDelimiters(WHITESPACE); - - String8 valueToken = mTokenizer->nextToken(WHITESPACE); - if (valueToken.find("\\", 0) >= 0 || valueToken.find("\"", 0) >= 0) { - ALOGE("%s: Found reserved character '\\' or '\"' in property value.", - mTokenizer->getLocation().string()); - return BAD_VALUE; - } - - mTokenizer->skipDelimiters(WHITESPACE); - if (!mTokenizer->isEol()) { - ALOGE("%s: Expected end of line, got '%s'.", - mTokenizer->getLocation().string(), - mTokenizer->peekRemainderOfLine().string()); - return BAD_VALUE; - } - - if (mMap->hasProperty(keyToken)) { - ALOGE("%s: Duplicate property value for key '%s'.", - mTokenizer->getLocation().string(), keyToken.string()); - return BAD_VALUE; - } - - mMap->addProperty(keyToken, valueToken); - } - - mTokenizer->nextLine(); - } - return OK; -} - -} // namespace android diff --git a/libutils/PropertyMap_fuzz.cpp b/libutils/PropertyMap_fuzz.cpp deleted file mode 100755 index fd50729f2..000000000 --- a/libutils/PropertyMap_fuzz.cpp +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright 2020 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. - */ - -#include "android-base/file.h" -#include "fuzzer/FuzzedDataProvider.h" -#include "utils/PropertyMap.h" -#include "utils/String8.h" - -static constexpr int MAX_FILE_SIZE = 256; -static constexpr int MAX_STR_LEN = 2048; -static constexpr int MAX_OPERATIONS = 1000; - -static const std::vector> - operations = { - [](FuzzedDataProvider*, android::PropertyMap propertyMap) -> void { - propertyMap.getProperties(); - }, - [](FuzzedDataProvider*, android::PropertyMap propertyMap) -> void { - propertyMap.clear(); - }, - [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void { - std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN); - android::String8 key = android::String8(keyStr.c_str()); - propertyMap.hasProperty(key); - }, - [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void { - std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN); - android::String8 key = android::String8(keyStr.c_str()); - android::String8 out; - propertyMap.tryGetProperty(key, out); - }, - [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void { - TemporaryFile tf; - // Generate file contents - std::string contents = dataProvider->ConsumeRandomLengthString(MAX_FILE_SIZE); - // If we have string contents, dump them into the file. - // Otherwise, just leave it as an empty file. - if (contents.length() > 0) { - const char* bytes = contents.c_str(); - android::base::WriteStringToFd(bytes, tf.fd); - } - android::PropertyMap* mapPtr = &propertyMap; - android::PropertyMap::load(android::String8(tf.path), &mapPtr); - }, - [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void { - std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN); - std::string valStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN); - android::String8 key = android::String8(keyStr.c_str()); - android::String8 val = android::String8(valStr.c_str()); - propertyMap.addProperty(key, val); - }, -}; -extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { - FuzzedDataProvider dataProvider(data, size); - android::PropertyMap proprtyMap = android::PropertyMap(); - - int opsRun = 0; - while (dataProvider.remaining_bytes() > 0 && opsRun++ < MAX_OPERATIONS) { - uint8_t op = dataProvider.ConsumeIntegralInRange(0, operations.size() - 1); - operations[op](&dataProvider, proprtyMap); - } - return 0; -} diff --git a/libutils/include/utils/PropertyMap.h b/libutils/include/utils/PropertyMap.h deleted file mode 100644 index a9e674f9a..000000000 --- a/libutils/include/utils/PropertyMap.h +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright (C) 2010 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. - */ - -#ifndef _UTILS_PROPERTY_MAP_H -#define _UTILS_PROPERTY_MAP_H - -#include -#include -#include -#include - -namespace android { - -/* - * Provides a mechanism for passing around string-based property key / value pairs - * and loading them from property files. - * - * The property files have the following simple structure: - * - * # Comment - * key = value - * - * Keys and values are any sequence of printable ASCII characters. - * The '=' separates the key from the value. - * The key and value may not contain whitespace. - * - * The '\' character is reserved for escape sequences and is not currently supported. - * The '"" character is reserved for quoting and is not currently supported. - * Files that contain the '\' or '"' character will fail to parse. - * - * The file must not contain duplicate keys. - * - * TODO Support escape sequences and quoted values when needed. - */ -class PropertyMap { -public: - /* Creates an empty property map. */ - PropertyMap(); - ~PropertyMap(); - - /* Clears the property map. */ - void clear(); - - /* Adds a property. - * Replaces the property with the same key if it is already present. - */ - void addProperty(const String8& key, const String8& value); - - /* Returns true if the property map contains the specified key. */ - bool hasProperty(const String8& key) const; - - /* Gets the value of a property and parses it. - * Returns true and sets outValue if the key was found and its value was parsed successfully. - * Otherwise returns false and does not modify outValue. (Also logs a warning.) - */ - bool tryGetProperty(const String8& key, String8& outValue) const; - bool tryGetProperty(const String8& key, bool& outValue) const; - bool tryGetProperty(const String8& key, int32_t& outValue) const; - bool tryGetProperty(const String8& key, float& outValue) const; - - /* Adds all values from the specified property map. */ - void addAll(const PropertyMap* map); - - /* Gets the underlying property map. */ - inline const KeyedVector& getProperties() const { return mProperties; } - - /* Loads a property map from a file. */ - static status_t load(const String8& filename, PropertyMap** outMap); - -private: - class Parser { - PropertyMap* mMap; - Tokenizer* mTokenizer; - - public: - Parser(PropertyMap* map, Tokenizer* tokenizer); - ~Parser(); - status_t parse(); - - private: - status_t parseType(); - status_t parseKey(); - status_t parseKeyProperty(); - status_t parseModifier(const String8& token, int32_t* outMetaState); - status_t parseCharacterLiteral(char16_t* outCharacter); - }; - - KeyedVector mProperties; -}; - -} // namespace android - -#endif // _UTILS_PROPERTY_MAP_H