am 4fe18615: am 56d57e88: am 5f130216: Merge "Handle errno properly to avoid corrupt str_parms"
* commit '4fe186159386a75ce17ca0b7ba6ace4294187cb2':
Handle errno properly to avoid corrupt str_parms
diff --git a/libcutils/Android.mk b/libcutils/Android.mk
index 93bccb0..635695a 100644
--- a/libcutils/Android.mk
+++ b/libcutils/Android.mk
@@ -91,6 +91,16 @@
LOCAL_CFLAGS += $(hostSmpFlag) -m64
include $(BUILD_HOST_STATIC_LIBRARY)
+# Tests for host
+# ========================================================
+include $(CLEAR_VARS)
+LOCAL_MODULE := tst_str_parms
+LOCAL_CFLAGS += -DTEST_STR_PARMS
+LOCAL_SRC_FILES := str_parms.c hashmap.c memory.c
+LOCAL_STATIC_LIBRARIES := liblog
+LOCAL_MODULE_TAGS := optional
+include $(BUILD_HOST_EXECUTABLE)
+
# Shared and static library for target
# ========================================================
diff --git a/libcutils/str_parms.c b/libcutils/str_parms.c
index 1edef11..d2b97e8 100644
--- a/libcutils/str_parms.c
+++ b/libcutils/str_parms.c
@@ -194,23 +194,46 @@
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);
- tmp_val = strdup(value);
- old_val = hashmapPut(str_parms->map, tmp_key, tmp_val);
-
- if (old_val) {
- free(old_val);
- free(tmp_key);
- } else if (errno == ENOMEM) {
- free(tmp_key);
- free(tmp_val);
- return -ENOMEM;
+ if (tmp_key == NULL) {
+ goto clean_up;
}
- return 0;
+
+ tmp_val = strdup(value);
+ if (tmp_val == NULL) {
+ goto clean_up;
+ }
+
+ 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;
+ }
+
+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)
@@ -341,7 +364,6 @@
{
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");
@@ -374,6 +396,15 @@
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