Return owned object from PropertyMap

To help keep track of object ownership, use smart pointers in
PropertyMap.

Bug: 163171599
Test: presubmit
Change-Id: I4194e6640c8b0e1ec0db9d9e65b3f6862d6f37d4

5 files changed, 29 insertions, 34 deletions

diff --git a/include/input/PropertyMap.h b/include/input/PropertyMap.h
index 3d0433133c..451918bb46 100644
--- a/include/input/PropertyMap.h
+++ b/include/input/PropertyMap.h
@@ -17,6 +17,7 @@
 #ifndef _UTILS_PROPERTY_MAP_H
 #define _UTILS_PROPERTY_MAP_H
 
+#include <android-base/result.h>
 #include <utils/Errors.h>
 #include <utils/KeyedVector.h>
 #include <utils/String8.h>
@@ -78,7 +79,7 @@ public:
     inline const KeyedVector<String8, String8>& getProperties() const { return mProperties; }
 
     /* Loads a property map from a file. */
-    static status_t load(const String8& filename, PropertyMap** outMap);
+    static android::base::Result<std::unique_ptr<PropertyMap>> load(const char* filename);
 
 private:
     class Parser {
diff --git a/libs/input/PropertyMap.cpp b/libs/input/PropertyMap.cpp
index 4833eb9c05..a842166761 100644
--- a/libs/input/PropertyMap.cpp
+++ b/libs/input/PropertyMap.cpp
@@ -107,23 +107,22 @@ void PropertyMap::addAll(const PropertyMap* map) {
     }
 }
 
-status_t PropertyMap::load(const String8& filename, PropertyMap** outMap) {
-    *outMap = nullptr;
+android::base::Result<std::unique_ptr<PropertyMap>> PropertyMap::load(const char* filename) {
+    std::unique_ptr<PropertyMap> outMap = std::make_unique<PropertyMap>();
+    if (outMap == nullptr) {
+        return android::base::Error(NO_MEMORY) << "Error allocating property map.";
+    }
 
-    Tokenizer* tokenizer;
-    status_t status = Tokenizer::open(filename, &tokenizer);
+    Tokenizer* rawTokenizer;
+    status_t status = Tokenizer::open(String8(filename), &rawTokenizer);
+    std::unique_ptr<Tokenizer> tokenizer(rawTokenizer);
     if (status) {
-        ALOGE("Error %d opening property file %s.", status, filename.string());
+        ALOGE("Error %d opening property file %s.", status, filename);
     } 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);
+            Parser parser(outMap.get(), tokenizer.get());
             status = parser.parse();
 #if DEBUG_PARSER_PERFORMANCE
             nsecs_t elapsedTime = systemTime(SYSTEM_TIME_MONOTONIC) - startTime;
@@ -132,14 +131,10 @@ status_t PropertyMap::load(const String8& filename, PropertyMap** outMap) {
                   elapsedTime / 1000000.0);
 #endif
             if (status) {
-                delete map;
-            } else {
-                *outMap = map;
+                return android::base::Error(BAD_VALUE) << "Could not parse " << filename;
             }
-        }
-        delete tokenizer;
     }
-    return status;
+    return std::move(outMap);
 }
 
 // --- PropertyMap::Parser ---
diff --git a/libs/input/PropertyMap_fuzz.cpp b/libs/input/PropertyMap_fuzz.cpp
index 23ead0e469..afb97a1d47 100755
--- a/libs/input/PropertyMap_fuzz.cpp
+++ b/libs/input/PropertyMap_fuzz.cpp
@@ -42,7 +42,7 @@ static const std::vector<std::function<void(FuzzedDataProvider*, android::Proper
                     android::String8 out;
                     propertyMap.tryGetProperty(key, out);
                 },
-                [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void {
+                [](FuzzedDataProvider* dataProvider, android::PropertyMap /*unused*/) -> void {
                     TemporaryFile tf;
                     // Generate file contents
                     std::string contents = dataProvider->ConsumeRandomLengthString(MAX_FILE_SIZE);
@@ -52,8 +52,7 @@ static const std::vector<std::function<void(FuzzedDataProvider*, android::Proper
                         const char* bytes = contents.c_str();
                         android::base::WriteStringToFd(bytes, tf.fd);
                     }
-                    android::PropertyMap* mapPtr = &propertyMap;
-                    android::PropertyMap::load(android::String8(tf.path), &mapPtr);
+                    android::PropertyMap::load(tf.path);
                 },
                 [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void {
                     std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN);
@@ -65,12 +64,12 @@ static const std::vector<std::function<void(FuzzedDataProvider*, android::Proper
 };
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     FuzzedDataProvider dataProvider(data, size);
-    android::PropertyMap proprtyMap = android::PropertyMap();
+    android::PropertyMap propertyMap = android::PropertyMap();
 
     int opsRun = 0;
     while (dataProvider.remaining_bytes() > 0 && opsRun++ < MAX_OPERATIONS) {
         uint8_t op = dataProvider.ConsumeIntegralInRange<uint8_t>(0, operations.size() - 1);
-        operations[op](&dataProvider, proprtyMap);
+        operations[op](&dataProvider, propertyMap);
     }
     return 0;
 }
diff --git a/services/inputflinger/host/InputDriver.cpp b/services/inputflinger/host/InputDriver.cpp
index cabc782b84..2ebdbcfcc4 100644
--- a/services/inputflinger/host/InputDriver.cpp
+++ b/services/inputflinger/host/InputDriver.cpp
@@ -36,7 +36,7 @@
 #define INDENT2 "    "
 
 struct input_property_map {
-    android::PropertyMap* propertyMap;
+    std::unique_ptr<android::PropertyMap> propertyMap;
 };
 
 struct input_property {
@@ -225,16 +225,17 @@ input_property_map_t* InputDriver::inputGetDevicePropertyMap(input_device_identi
         ALOGD("No input device configuration file found for device '%s'.",
                 idi.name.c_str());
     } else {
-        auto propMap = new input_property_map_t();
-        status_t status = PropertyMap::load(String8(configFile.c_str()), &propMap->propertyMap);
-        if (status) {
+        std::unique_ptr<input_property_map_t> propMap = std::make_unique<input_property_map_t>();
+        android::base::Result<std::unique_ptr<PropertyMap>> result =
+                PropertyMap::load(configFile.c_str());
+        if (!result.ok()) {
             ALOGE("Error loading input device configuration file for device '%s'. "
                     "Using default configuration.",
                     idi.name.c_str());
-            delete propMap;
             return nullptr;
         }
-        return propMap;
+        propMap->propertyMap = std::move(*result);
+        return propMap.release();
     }
     return nullptr;
 }
@@ -278,7 +279,6 @@ void InputDriver::inputFreeDeviceProperty(input_property_t* property) {
 
 void InputDriver::inputFreeDevicePropertyMap(input_property_map_t* map) {
     if (map != nullptr) {
-        delete map->propertyMap;
         delete map;
     }
 }
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp
index 67d3cc2115..c5210b53ae 100644
--- a/services/inputflinger/reader/EventHub.cpp
+++ b/services/inputflinger/reader/EventHub.cpp
@@ -280,14 +280,14 @@ void EventHub::Device::loadConfigurationLocked() {
     if (configurationFile.empty()) {
         ALOGD("No input device configuration file found for device '%s'.", identifier.name.c_str());
     } else {
-        PropertyMap* propertyMap;
-        status_t status = PropertyMap::load(String8(configurationFile.c_str()), &propertyMap);
-        if (status) {
+        android::base::Result<std::unique_ptr<PropertyMap>> propertyMap =
+                PropertyMap::load(configurationFile.c_str());
+        if (!propertyMap.ok()) {
             ALOGE("Error loading input device configuration file for device '%s'.  "
                   "Using default configuration.",
                   identifier.name.c_str());
         } else {
-            configuration = std::unique_ptr<PropertyMap>(propertyMap);
+            configuration = std::move(*propertyMap);
         }
     }
 }