Return owned object from PropertyMap
To help keep track of object ownership, use smart pointers in
PropertyMap.
Bug: 163171599
Test: presubmit
Change-Id: I4194e6640c8b0e1ec0db9d9e65b3f6862d6f37d4
diff --git a/include/input/PropertyMap.h b/include/input/PropertyMap.h
index 3d04331..451918b 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 @@
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 4833eb9..a842166 100644
--- a/libs/input/PropertyMap.cpp
+++ b/libs/input/PropertyMap.cpp
@@ -107,23 +107,22 @@
}
}
-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 @@
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 23ead0e..afb97a1 100755
--- a/libs/input/PropertyMap_fuzz.cpp
+++ b/libs/input/PropertyMap_fuzz.cpp
@@ -42,7 +42,7 @@
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 @@
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 @@
};
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 cabc782..2ebdbcf 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 @@
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::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 67d3cc2..c5210b5 100644
--- a/services/inputflinger/reader/EventHub.cpp
+++ b/services/inputflinger/reader/EventHub.cpp
@@ -280,14 +280,14 @@
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);
}
}
}