1 files changed, 44 insertions, 13 deletions

diff --git a/libcutils/str_parms.c b/libcutils/str_parms.c
index 7cfbcb3..953b91d 100644
--- a/libcutils/str_parms.c
+++ b/libcutils/str_parms.c
@@ -194,23 +194,46 @@ err_create_str_parms:
 int str_parms_add_str(struct str_parms *str_parms, const char *key,
                       const char *value)
 {
-    void *old_val;
-    void *tmp_key;
-    void *tmp_val;
+    void *tmp_key = NULL;
+    void *tmp_val = NULL;
+    void *old_val = NULL;
+
+    // strdup and hashmapPut both set errno on failure.
+    // Set errno to 0 so we can recognize whether anything went wrong.
+    int saved_errno = errno;
+    errno = 0;
 
     tmp_key = strdup(key);
+    if (tmp_key == NULL) {
+        goto clean_up;
+    }
+
     tmp_val = strdup(value);
-    old_val = hashmapPut(str_parms->map, tmp_key, tmp_val);
+    if (tmp_val == NULL) {
+        goto clean_up;
+    }
 
-    if (old_val) {
-        free(old_val);
-        free(tmp_key);
-    } else if (errno == ENOMEM) {
-        free(tmp_key);
-        free(tmp_val);
-        return -ENOMEM;
+    old_val = hashmapPut(str_parms->map, tmp_key, tmp_val);
+    if (old_val == NULL) {
+        // Did hashmapPut fail?
+        if (errno == ENOMEM) {
+            goto clean_up;
+        }
+        // For new keys, hashmap takes ownership of tmp_key and tmp_val.
+        tmp_key = tmp_val = NULL;
+    } else {
+        // For existing keys, hashmap takes ownership of tmp_val.
+        // (It also gives up ownership of old_val.)
+        tmp_val = NULL;
     }
-    return 0;
+
+clean_up:
+    free(tmp_key);
+    free(tmp_val);
+    free(old_val);
+    int result = -errno;
+    errno = saved_errno;
+    return result;
 }
 
 int str_parms_add_int(struct str_parms *str_parms, const char *key, int value)
@@ -337,7 +360,6 @@ static void test_str_parms_str(const char *str)
 {
     struct str_parms *str_parms;
     char *out_str;
-    int ret;
 
     str_parms = str_parms_create_str(str);
     str_parms_add_str(str_parms, "dude", "woah");
@@ -370,6 +392,15 @@ int main(void)
     test_str_parms_str("foo=bar;baz=bat;");
     test_str_parms_str("foo=bar;baz=bat;foo=bar");
 
+    // hashmapPut reports errors by setting errno to ENOMEM.
+    // Test that we're not confused by running in an environment where this is already true.
+    errno = ENOMEM;
+    test_str_parms_str("foo=bar;baz=");
+    if (errno != ENOMEM) {
+        abort();
+    }
+    test_str_parms_str("foo=bar;baz=");
+
     return 0;
 }
 #endif