eina_value: break usage, but makes it more uniform and correct.

I did a bad decision to steal memory for Array, List, Hash and Struct
types, it was nice to not have to copy it internally, but breaks when
one needs to set a new value that was set elsewhere. What did not
happen with string, integers and other basic types.

This was exposed by Raphael Kubo using eina_model_property_set() with
complex types (Array, List and Hash) and it was not possible to
correctly set such properties.

Now it's all set, but the behavior changed and the memory is not
stolen and released anymore. Test eina_test_value.c was changed to
reflect it.



SVN revision: 67843

legacy/eina/src/include/eina_model.h

@@ -1533,14 +1533,14 @@ EAPI extern const Eina_Model_Interface *EINA_MODEL_INTERFACE_PROPERTIES_STRUCT;
  *
  * @param model The model instance to configure.
  * @param desc The structure description to use.
- * @param memory If not @c NULL, will be adopted by model.
+ * @param memory If not @c NULL, will be copied by model.
  * @return #EINA_TRUE on success, #EINA_FALSE on failure.
  *
  * This will setup the internal pointers so that the given @a desc is
  * used to manage the properties of this struct.
  *
- * If a given memory is provided, it will be adopted (not copied!),
+ * If a given memory is provided, it will be copied (including
- * being free'd when the model is gone.
+ * members) and no references are taken after this function returns.
  *
  * @see #EINA_VALUE_TYPE_STRUCT
  *

legacy/eina/src/include/eina_value.h

@@ -215,10 +215,11 @@ EAPI extern const Eina_Value_Type *EINA_VALUE_TYPE_STRING;
  *  @li eina_value_array_pget() and eina_value_array_pset()
  *
  * eina_value_set() takes an #Eina_Value_Array where just @c subtype
- * and @c step are used. If there is an @c array, it will be adopted
+ * and @c step are used. If there is an @c array, it will be copied
- * and its contents must be properly configurable as @c subtype
+ * (including each item) and its contents must be properly
- * expects. eina_value_pset() takes a pointer to an #Eina_Value_Array.
+ * configurable as @c subtype expects. eina_value_pset() takes a
- * For your convenience, use eina_value_array_setup().
+ * pointer to an #Eina_Value_Array.  For your convenience, use
+ * eina_value_array_setup().
  *
  * eina_value_get() and eina_value_pget() takes a pointer to
  * #Eina_Value_Array, it's an exact copy of the current structure in
@@ -237,10 +238,11 @@ EAPI extern const Eina_Value_Type *EINA_VALUE_TYPE_ARRAY;
  *  @li eina_value_list_pget() and eina_value_list_pset()
  *
  * eina_value_set() takes an #Eina_Value_List where just @c subtype is
- * used. If there is an @c list, it will be adopted and its contents
+ * used. If there is an @c list, it will be copied (including each
- * must be properly configurable as @c subtype
+ * item) and its contents must be properly configurable as @c
- * expects. eina_value_pset() takes a pointer to an #Eina_Value_List.
+ * subtype expects. eina_value_pset() takes a pointer to an
- * For your convenience, use eina_value_list_setup().
+ * #Eina_Value_List.  For your convenience, use
+ * eina_value_list_setup().
  *
  * eina_value_get() and eina_value_pget() takes a pointer to
  * #Eina_Value_List, it's an exact copy of the current structure in
@@ -260,9 +262,9 @@ EAPI extern const Eina_Value_Type *EINA_VALUE_TYPE_LIST;
  *
  * eina_value_set() takes an #Eina_Value_Hash where just @c subtype
  * and @c buckets_power_size are used. If there is an @c hash, it will
- * be adopted and its contents must be properly configurable as @c
+ * be copied (including each item) and its contents must be
- * subtype expects. eina_value_pset() takes a pointer to an
+ * properly configurable as @c subtype expects. eina_value_pset()
- * #Eina_Value_Hash.  For your convenience, use
+ * takes a pointer to an #Eina_Value_Hash.  For your convenience, use
  * eina_value_hash_setup().
  *
  * eina_value_get() and eina_value_pget() takes a pointer to
@@ -319,9 +321,10 @@ EAPI extern const Eina_Value_Type *EINA_VALUE_TYPE_BLOB;
  *  @li eina_value_struct_pget() and eina_value_struct_pset()
  *
  * eina_value_set() takes an #Eina_Value_Struct where just @c desc is
- * used. If there is an @c memory, it will be adopted and its contents
+ * used. If there is an @c memory, it will be copied (including each
- * must be properly configurable as @c desc expects. eina_value_pset()
+ * member) and its contents must be properly configurable as @c desc
- * takes a pointer to an #Eina_Value_Struct.  For your convenience, use
+ * expects. eina_value_pset() takes a pointer to an
+ * #Eina_Value_Struct.  For your convenience, use
  * eina_value_struct_setup().
  *
  * eina_value_get() and eina_value_pget() takes a pointer to

legacy/eina/src/lib/eina_value.c

@@ -2626,7 +2626,7 @@ _eina_value_type_array_convert_from(const Eina_Value_Type *type, const Eina_Valu
 }
 
 static Eina_Bool
-_eina_value_type_array_pset(const Eina_Value_Type *type __UNUSED__, void *mem, const void *ptr)
+_eina_value_type_array_pset(const Eina_Value_Type *type, void *mem, const void *ptr)
 {
    Eina_Value_Array *tmem = mem;
    const Eina_Value_Array *desc = ptr;
@@ -2639,6 +2639,8 @@ _eina_value_type_array_pset(const Eina_Value_Type *type __UNUSED__, void *mem, c
    desc_array = desc->array;
    if (desc_array)
      {
+        Eina_Value_Array tmp;
+
         EINA_SAFETY_ON_FALSE_RETURN_VAL
           (desc_array->member_size == desc->subtype->value_size, EINA_FALSE);
 
@@ -2647,29 +2649,28 @@ _eina_value_type_array_pset(const Eina_Value_Type *type __UNUSED__, void *mem, c
              tmem->subtype = desc->subtype;
              return EINA_TRUE;
           }
+
+        if (!_eina_value_type_array_copy(type, desc, &tmp))
+          return EINA_FALSE;
+
+        _eina_value_type_array_flush(type, tmem);
+        memcpy(tmem, &tmp, sizeof(tmp));
+        return EINA_TRUE;
      }
 
    if (tmem->array)
      {
         _eina_value_type_array_flush_elements(tmem);
-        if (desc_array)
+        eina_inarray_setup(tmem->array, desc->subtype->value_size, desc->step);
-          eina_inarray_free(tmem->array);
-        else
-          eina_inarray_setup(tmem->array, desc->subtype->value_size,
-                             desc->step);
      }
-   else if (!desc_array)
+   else
      {
         tmem->array = eina_inarray_new(desc->subtype->value_size, desc->step);
         if (!tmem->array)
           return EINA_FALSE;
      }
 
-   if (desc_array)
-     tmem->array = desc_array;
-
    tmem->subtype = desc->subtype;
-
    return EINA_TRUE;
 }
 
@@ -2959,7 +2960,7 @@ _eina_value_type_list_convert_from(const Eina_Value_Type *type, const Eina_Value
 }
 
 static Eina_Bool
-_eina_value_type_list_pset(const Eina_Value_Type *type __UNUSED__, void *mem, const void *ptr)
+_eina_value_type_list_pset(const Eina_Value_Type *type, void *mem, const void *ptr)
 {
    Eina_Value_List *tmem = mem;
    const Eina_Value_List *desc = ptr;
@@ -2974,10 +2975,21 @@ _eina_value_type_list_pset(const Eina_Value_Type *type __UNUSED__, void *mem, co
         return EINA_TRUE;
      }
 
-   _eina_value_type_list_flush_elements(tmem);
+   if (desc->list)
-   tmem->subtype = desc->subtype;
+     {
-   tmem->list = desc->list;
+        Eina_Value_List tmp;
 
+        if (!_eina_value_type_list_copy(type, desc, &tmp))
+          return EINA_FALSE;
+
+        _eina_value_type_list_flush(type, tmem);
+        memcpy(tmem, &tmp, sizeof(tmp));
+        return EINA_TRUE;
+     }
+
+   _eina_value_type_list_flush_elements(tmem);
+
+   tmem->subtype = desc->subtype;
    return EINA_TRUE;
 }
 
@@ -3323,7 +3335,7 @@ _eina_value_type_hash_convert_to(const Eina_Value_Type *type __UNUSED__, const E
 }
 
 static Eina_Bool
-_eina_value_type_hash_pset(const Eina_Value_Type *type __UNUSED__, void *mem, const void *ptr)
+_eina_value_type_hash_pset(const Eina_Value_Type *type, void *mem, const void *ptr)
 {
    Eina_Value_Hash *tmem = mem;
    const Eina_Value_Hash *desc = ptr;
@@ -3338,14 +3350,23 @@ _eina_value_type_hash_pset(const Eina_Value_Type *type __UNUSED__, void *mem, co
         return EINA_TRUE;
      }
 
+   if (desc->hash)
+     {
+        Eina_Value_Hash tmp;
+
+        if (!_eina_value_type_hash_copy(type, desc, &tmp))
+          return EINA_FALSE;
+
+        _eina_value_type_hash_flush(type, tmem);
+        memcpy(tmem, &tmp, sizeof(tmp));
+        return EINA_TRUE;
+     }
+
    if (tmem->hash) _eina_value_type_hash_flush_elements(tmem);
 
-   if (desc->hash)
-     tmem->hash = desc->hash;
-   else if (!_eina_value_type_hash_create(tmem))
-     return EINA_FALSE;
-
    tmem->subtype = desc->subtype;
+   if (!_eina_value_type_hash_create(tmem))
+     return EINA_FALSE;
 
    return EINA_TRUE;
 }
@@ -4333,10 +4354,21 @@ _eina_value_type_struct_pset(const Eina_Value_Type *type, void *mem, const void
         return EINA_TRUE;
      }
 
-   _eina_value_type_struct_flush(type, mem);
+   if (desc->memory)
+     {
+        Eina_Value_Struct tmp;
 
-   *tmem = *desc;
+        if (!_eina_value_type_struct_copy(type, desc, &tmp))
-   if (tmem->memory) return EINA_TRUE;
+          return EINA_FALSE;
+
+        _eina_value_type_struct_flush(type, tmem);
+        memcpy(tmem, &tmp, sizeof(tmp));
+        return EINA_TRUE;
+     }
+
+   if (tmem->memory) _eina_value_type_struct_flush(type, mem);
+
+   tmem->desc = desc->desc;
 
    ops = _eina_value_type_struct_ops_get(desc);
    if ((ops) && (ops->alloc))
@@ -4377,6 +4409,8 @@ _eina_value_type_struct_pset(const Eina_Value_Type *type, void *mem, const void
      ops->free(ops, tmem->desc, tmem->memory);
    else
      free(tmem->memory);
+   tmem->memory = NULL;
+   tmem->desc = NULL;
    return EINA_FALSE;
 }
 

legacy/eina/src/tests/eina_test_model.c

@@ -855,6 +855,127 @@ START_TEST(eina_model_test_struct)
 }
 END_TEST
 
+static Eina_Bool
+_struct_complex_members_constructor(Eina_Model *m)
+{
+   struct myst {
+      Eina_Value_Array a;
+      Eina_Value_List l;
+      Eina_Value_Hash h;
+      Eina_Value_Struct s;
+   } *st;
+   struct subst {
+      int i, j;
+   };
+   static Eina_Value_Struct_Member myst_members[] = {
+        EINA_VALUE_STRUCT_MEMBER(NULL, struct myst, a),
+        EINA_VALUE_STRUCT_MEMBER(NULL, struct myst, l),
+        EINA_VALUE_STRUCT_MEMBER(NULL, struct myst, h),
+        EINA_VALUE_STRUCT_MEMBER(NULL, struct myst, s)
+   };
+   static Eina_Value_Struct_Desc myst_desc = {
+     EINA_VALUE_STRUCT_DESC_VERSION,
+     NULL, myst_members, EINA_C_ARRAY_LENGTH(myst_members), sizeof(struct myst)
+   };
+   static Eina_Value_Struct_Member subst_members[] = {
+        EINA_VALUE_STRUCT_MEMBER(NULL, struct subst, i),
+        EINA_VALUE_STRUCT_MEMBER(NULL, struct subst, j)
+   };
+   static Eina_Value_Struct_Desc subst_desc = {
+     EINA_VALUE_STRUCT_DESC_VERSION,
+     NULL, subst_members, EINA_C_ARRAY_LENGTH(subst_members),
+     sizeof(struct subst)
+   };
+
+   if (!myst_members[0].type)
+     {
+        myst_members[0].type = EINA_VALUE_TYPE_ARRAY;
+        myst_members[1].type = EINA_VALUE_TYPE_LIST;
+        myst_members[2].type = EINA_VALUE_TYPE_HASH;
+        myst_members[3].type = EINA_VALUE_TYPE_STRUCT;
+     }
+
+   if (!subst_members[0].type)
+     {
+        subst_members[0].type = EINA_VALUE_TYPE_INT;
+        subst_members[1].type = EINA_VALUE_TYPE_INT;
+     }
+
+   if (!eina_model_type_constructor(EINA_MODEL_TYPE_STRUCT, m))
+     return EINA_FALSE;
+
+   st = calloc(1, sizeof(*st));
+   if (!st)
+     {
+        eina_error_set(EINA_ERROR_OUT_OF_MEMORY);
+        return EINA_FALSE;
+     }
+
+   st->a.subtype = EINA_VALUE_TYPE_STRING;
+   st->l.subtype = EINA_VALUE_TYPE_STRING;
+   st->h.subtype = EINA_VALUE_TYPE_STRING;
+   st->s.desc = &subst_desc;
+
+   if (!eina_model_struct_set(m, &myst_desc, st))
+     {
+        free(st);
+        return EINA_FALSE;
+     }
+
+   return EINA_TRUE;
+}
+
+START_TEST(eina_model_test_struct_complex_members)
+{
+   Eina_Model *m;
+   Eina_Value outv;
+   char *s;
+   Eina_Model_Type type = EINA_MODEL_TYPE_INIT_NOPRIVATE
+     ("struct_complex_members", Eina_Model_Type, NULL, NULL, NULL);
+
+   eina_init();
+
+   type.constructor = _struct_complex_members_constructor;
+   type.parent = EINA_MODEL_TYPE_STRUCT;
+
+   m = eina_model_new(&type);
+   fail_unless(m != NULL);
+
+   fail_unless(eina_model_property_get(m, "a", &outv));
+   fail_unless(eina_value_array_append(&outv, "Hello"));
+   fail_unless(eina_value_array_append(&outv, "World"));
+   fail_unless(eina_model_property_set(m, "a", &outv));
+   eina_value_flush(&outv);
+
+   fail_unless(eina_model_property_get(m, "l", &outv));
+   fail_unless(eina_value_list_append(&outv, "Some"));
+   fail_unless(eina_value_list_append(&outv, "Thing"));
+   fail_unless(eina_model_property_set(m, "l", &outv));
+   eina_value_flush(&outv);
+
+   fail_unless(eina_model_property_get(m, "h", &outv));
+   fail_unless(eina_value_hash_set(&outv, "key", "value"));
+   fail_unless(eina_model_property_set(m, "h", &outv));
+   eina_value_flush(&outv);
+
+   fail_unless(eina_model_property_get(m, "s", &outv));
+   fail_unless(eina_value_struct_set(&outv, "i", 1234));
+   fail_unless(eina_value_struct_set(&outv, "j", 44));
+   fail_unless(eina_model_property_set(m, "s", &outv));
+   eina_value_flush(&outv);
+
+   s = eina_model_to_string(m);
+   fail_unless(s != NULL);
+   ck_assert_str_eq(s, "struct_complex_members({a: [Hello, World], h: {key: value}, l: [Some, Thing], s: {i: 1234, j: 44}}, [])");
+   free(s);
+
+   ck_assert_int_eq(eina_model_refcount(m), 1);
+
+   eina_model_unref(m);
+   eina_shutdown();
+}
+END_TEST
+
 void
 eina_test_model(TCase *tc)
 {
@@ -867,4 +988,5 @@ eina_test_model(TCase *tc)
    tcase_add_test(tc, eina_model_test_child_sorted_iterator);
    tcase_add_test(tc, eina_model_test_child_filtered_iterator);
    tcase_add_test(tc, eina_model_test_struct);
+   tcase_add_test(tc, eina_model_test_struct_complex_members);
 }

legacy/eina/src/tests/eina_test_value.c

@@ -1140,8 +1140,10 @@ START_TEST(eina_value_test_array)
    fail_unless(eina_inarray_append(inarray, &c) >= 0);
    desc.subtype = EINA_VALUE_TYPE_CHAR;
    desc.step = 0;
-   desc.array = inarray; /* will be adopted and freed by value */
+   desc.array = inarray;
    fail_unless(eina_value_set(value, desc)); /* manually configure */
+   eina_inarray_free(inarray);
+
    fail_unless(eina_value_array_get(value, 0, &c));
    fail_unless(c == 11);
    fail_unless(eina_value_array_get(value, 1, &c));
@@ -1242,11 +1244,13 @@ START_TEST(eina_value_test_list)
 
    desc.subtype = EINA_VALUE_TYPE_STRING;
    desc.list = NULL;
-   desc.list = eina_list_append(desc.list, strdup("hello"));
+   desc.list = eina_list_append(desc.list, "hello");
-   desc.list = eina_list_append(desc.list, strdup("world"));
+   desc.list = eina_list_append(desc.list, "world");
-   desc.list = eina_list_append(desc.list, strdup("eina"));
+   desc.list = eina_list_append(desc.list, "eina");
    fail_unless(eina_list_count(desc.list) == 3);
    fail_unless(eina_value_set(value, desc));
+   eina_list_free(desc.list);
+
    fail_unless(eina_value_list_get(value, 0, &s));
    fail_unless(s != NULL);
    fail_unless(strcmp(s, "hello") == 0);
@@ -1351,14 +1355,17 @@ START_TEST(eina_value_test_hash)
    fail_unless(desc.hash != NULL);
    /* watch out hash pointer is to a size of subtype->value_size! */
    ptr = malloc(sizeof(char *));
-   *ptr = strdup("there");
+   *ptr = "there";
    fail_unless(eina_hash_add(desc.hash, "hi", ptr));
    ptr = malloc(sizeof(char *));
-   *ptr = strdup("y");
+   *ptr = "y";
    fail_unless(eina_hash_add(desc.hash, "x", ptr));
-
    fail_unless(eina_value_set(value, desc));
 
+   free(eina_hash_find(desc.hash, "hi"));
+   free(eina_hash_find(desc.hash, "x"));
+   eina_hash_free(desc.hash);
+
    fail_unless(eina_value_hash_get(value, "hi", &s));
    fail_unless(s != NULL);
    fail_unless(strcmp(s, "there") == 0);
@@ -1755,20 +1762,17 @@ START_TEST(eina_value_test_array_of_struct)
    for (i = 0; i < 10; i++)
      {
         Eina_Value_Struct desc;
-        struct myst *st;
+        struct myst st;
         char buf[64];
 
         snprintf(buf, sizeof(buf), "item%02d", i);
-        st = malloc(sizeof(struct myst));
+        st.a = i;
-        fail_unless(st != NULL);
+        st.b = i * 10;
-        st->a = i;
+        st.c = i * 100;
-        st->b = i * 10;
+        st.s = buf;
-        st->c = i * 100;
-        st->s = strdup(buf);
-        fail_unless(st->s != NULL);
 
         desc.desc = &myst_desc;
-        desc.memory = st;
+        desc.memory = &st;
         fail_unless(eina_value_array_append(value, desc));
      }