From f8a033fd70ac8a07c3c88ba2d32d61912f9961b5 Mon Sep 17 00:00:00 2001 From: Lauro Moura Date: Mon, 23 Apr 2018 16:20:12 -0300 Subject: [PATCH] efl_mono: Add support for Eina.Error/Empty in eina.Value Summary: eina.Value.Empty now means that we have an zeroed (empty) eina value. For optional values that are empty use eina.Value.OptionalEmpty. This was required to support the empty values passed with EINA_VALUE_EMPTY in some Ecore futures. Also, returning an eina_value by value is not supported in eolian for safety reasons, so we removed some tests that tried to use this behavior. Depends on D6171 Reviewers: felipealmeida Reviewed By: felipealmeida Subscribers: cedric, zmike Tags: #efl Differential Revision: https://phab.enlightenment.org/D6172 --- .../eolian/mono/struct_definition.hh | 2 +- src/bindings/mono/eina_mono/eina_common.cs | 7 ++ src/bindings/mono/eina_mono/eina_value.cs | 118 ++++++++++++++++-- src/lib/efl_mono/efl_custom_exports_mono.c | 13 ++ src/tests/efl_mono/Value.cs | 67 ++++++---- src/tests/efl_mono/ValueEolian.cs | 17 --- src/tests/efl_mono/libefl_mono_native_test.c | 8 -- 7 files changed, 172 insertions(+), 60 deletions(-) diff --git a/src/bin/eolian_mono/eolian/mono/struct_definition.hh b/src/bin/eolian_mono/eolian/mono/struct_definition.hh index 79e6ec6f1b..618b433213 100644 --- a/src/bin/eolian_mono/eolian/mono/struct_definition.hh +++ b/src/bin/eolian_mono/eolian/mono/struct_definition.hh @@ -240,7 +240,7 @@ struct to_internal_field_convert_generator else if (field.type.c_type == "Eina_Value *" || field.type.c_type == "const Eina_Value *") { if (!as_generator( - scope_tab << scope_tab << "_internal_struct." << string << " = _external_struct." << string << ".Handle;\n" + scope_tab << scope_tab << "_internal_struct." << string << " = _external_struct." << string << ".NativeHandle;\n" ).generate(sink, std::make_tuple(field_name, field_name), context)) return false; } diff --git a/src/bindings/mono/eina_mono/eina_common.cs b/src/bindings/mono/eina_mono/eina_common.cs index fd9d888bb5..2a9766cd4d 100644 --- a/src/bindings/mono/eina_mono/eina_common.cs +++ b/src/bindings/mono/eina_mono/eina_common.cs @@ -22,6 +22,8 @@ internal static class NativeCustomExportFunctions efl_mono_native_free_ref(IntPtr ptr); [DllImport(efl.Libs.CustomExports)] public static extern IntPtr efl_mono_native_alloc(uint count); + [DllImport(efl.Libs.CustomExports)] public static extern IntPtr + efl_mono_native_memset(IntPtr ptr, uint fill, uint count); [DllImport(efl.Libs.CustomExports)] public static extern IntPtr efl_mono_native_alloc_copy(IntPtr val, uint size); [DllImport(efl.Libs.CustomExports)] public static extern IntPtr @@ -56,6 +58,11 @@ public static class MemoryNative { return NativeCustomExportFunctions.efl_mono_native_alloc(Convert.ToUInt32(count)); } + public static void Memset(IntPtr ptr, int fill, int count) + { + NativeCustomExportFunctions.efl_mono_native_memset(ptr, Convert.ToUInt32(fill), Convert.ToUInt32(count)); + } + public static IntPtr AllocCopy(IntPtr ptr, int count) { return NativeCustomExportFunctions.efl_mono_native_alloc_copy(ptr, Convert.ToUInt32(count)); diff --git a/src/bindings/mono/eina_mono/eina_value.cs b/src/bindings/mono/eina_mono/eina_value.cs index 50ff9457ef..578c660b91 100644 --- a/src/bindings/mono/eina_mono/eina_value.cs +++ b/src/bindings/mono/eina_mono/eina_value.cs @@ -325,7 +325,7 @@ static internal class UnsafeNativeMethods { [return: MarshalAsAttribute(UnmanagedType.U1)] internal static extern bool eina_value_pset_wrapper(IntPtr handle, ref eina.EinaNative.Value_List ptr); - [DllImport(efl.Libs.CustomExports)] + [DllImport(efl.Libs.Eina)] [return: MarshalAsAttribute(UnmanagedType.U1)] internal static extern bool eina_value_copy(IntPtr src, IntPtr dest); @@ -379,6 +379,10 @@ static internal class UnsafeNativeMethods { // Optional [DllImport(efl.Libs.CustomExports)] internal static extern IntPtr type_optional(); + + // Error + [DllImport(efl.Libs.CustomExports)] + internal static extern IntPtr type_error(); } } @@ -386,8 +390,8 @@ static internal class UnsafeNativeMethods { [StructLayout(LayoutKind.Sequential)] public struct Value_Native { - IntPtr type; - IntPtr value; // Atually an Eina_Value_Union, but it is padded to 8 bytes. + public IntPtr Type; + public IntPtr Value; // Atually an Eina_Value_Union, but it is padded to 8 bytes. } @@ -470,6 +474,10 @@ public enum ValueType { Hash, /// Optional (aka empty) values. Optional, + /// Error values. + Error, + /// Empty values. + Empty, } static class ValueTypeMethods { @@ -521,6 +529,11 @@ static class ValueTypeMethods { return val == ValueType.Optional; } + public static bool IsError(this ValueType val) + { + return val == ValueType.Error; + } + /// Returns the Marshal.SizeOf for the given ValueType native structure. public static int MarshalSizeOf(this ValueType val) { @@ -608,6 +621,12 @@ static class ValueTypeBridge ManagedToNative.Add(ValueType.Optional, type_optional()); NativeToManaged.Add(type_optional(), ValueType.Optional); + ManagedToNative.Add(ValueType.Error, type_error()); + NativeToManaged.Add(type_error(), ValueType.Error); + + ManagedToNative.Add(ValueType.Empty, IntPtr.Zero); + NativeToManaged.Add(IntPtr.Zero, ValueType.Empty); + TypesLoaded = true; } } @@ -647,10 +666,13 @@ public class Value : IDisposable, IComparable, IEquatable // EINA_VALUE_TYPE_STRUCT: Eina_Value_Struct -- FIXME - public IntPtr Handle { get; protected set;} + internal IntPtr Handle { get; set;} + /// Whether this wrapper owns (can free) the native value. public Ownership Ownership { get; protected set;} private bool Disposed; + /// Whether this wrapper has already freed the native value. public bool Flushed { get; protected set;} + /// Whether this is an Optional value (meaning it can have a value or not). public bool Optional { get { return GetValueType() == eina.ValueType.Optional; @@ -663,7 +685,16 @@ public class Value : IDisposable, IComparable, IEquatable // } } */ } + /// Whether this wrapper is actually empty/uninitialized (zeroed). This is different from an empty optional value. public bool Empty { + get { + SanityChecks(); + return GetValueType() == eina.ValueType.Empty; + } + } + + /// Whether this optional value is empty. + public bool OptionalEmpty { get { OptionalSanityChecks(); bool empty; @@ -690,7 +721,14 @@ public class Value : IDisposable, IComparable, IEquatable { if (type.IsContainer()) throw new ArgumentException("To use container types you must provide a subtype"); + this.Handle = MemoryNative.Alloc(eina_value_sizeof()); + if (this.Handle == IntPtr.Zero) + throw new OutOfMemoryException("Failed to allocate memory for eina.Value"); + + // Initialize to EINA_VALUE_EMPTY before performing any other operation on this value. + MemoryNative.Memset(this.Handle, 0, eina_value_sizeof()); + this.Ownership = Ownership.Managed; Setup(type); } @@ -710,12 +748,22 @@ public class Value : IDisposable, IComparable, IEquatable /// Constructor to build value from Values_Natives passed by value from C. public Value(Value_Native value) { - this.Handle = MemoryNative.Alloc(Marshal.SizeOf(typeof(Value_Native))); IntPtr tmp = MemoryNative.Alloc(Marshal.SizeOf(typeof(Value_Native))); try { Marshal.StructureToPtr(value, tmp, false); // Can't get the address of a struct directly. - if (!eina_value_copy(tmp, this.Handle)) - throw new System.InvalidOperationException("Failed to copy value to managed memory."); + if (value.Type == IntPtr.Zero) // Got an EINA_VALUE_EMPTY by value. + { + this.Handle = tmp; + tmp = IntPtr.Zero; + } + else + { + this.Handle = MemoryNative.Alloc(Marshal.SizeOf(typeof(Value_Native))); + + // Copy is used to deep copy the pointed payload (e.g. strings) inside this struct, so we can own this value. + if (!eina_value_copy(tmp, this.Handle)) + throw new System.InvalidOperationException("Failed to copy value to managed memory."); + } } catch { MemoryNative.Free(this.Handle); throw; @@ -786,7 +834,7 @@ public class Value : IDisposable, IComparable, IEquatable } if (!Disposed && (Handle != IntPtr.Zero)) { - if (!Flushed) + if (!Flushed && !Empty) eina_value_flush_wrapper(this.Handle); MemoryNative.Free(this.Handle); @@ -801,11 +849,12 @@ public class Value : IDisposable, IComparable, IEquatable } /// Returns the native handle wrapped by this object. - public IntPtr NativeHandle() - { - if (Disposed) - throw new ObjectDisposedException(base.GetType().Name); - return this.Handle; + public IntPtr NativeHandle { + get { + if (Disposed) + throw new ObjectDisposedException(base.GetType().Name); + return this.Handle; + } } /// Converts this storage to type 'type' @@ -814,6 +863,18 @@ public class Value : IDisposable, IComparable, IEquatable if (Disposed) throw new ObjectDisposedException(base.GetType().Name); + // Can't call setup with Empty value type (would give an eina error) + if (type == eina.ValueType.Empty) + { + // Need to cleanup as it may point to payload outside the underlying Eina_Value (like arrays and strings). + if (!Empty) + eina_value_flush_wrapper(this.Handle); + + MemoryNative.Memset(this.Handle, 0, eina_value_sizeof()); + + return true; + } + if (type.IsContainer()) throw new ArgumentException("To setup a container you must provide a subtype."); @@ -1075,6 +1136,21 @@ public class Value : IDisposable, IComparable, IEquatable return eina_value_set_wrapper_string(this.Handle, value); } + /// Stores the given error value. + public bool Set(eina.Error value) + { + SanityChecks(); + + int error_code = value; + + if (this.Optional) + return eina_value_optional_pset(this.Handle, + ValueTypeBridge.GetNative(ValueType.Error), + ref error_code); + + return eina_value_set_wrapper_int(this.Handle, error_code); + } + /// Stores the given value into this value. The target value must be an optional. public bool Set(Value value) { @@ -1228,6 +1304,22 @@ public class Value : IDisposable, IComparable, IEquatable return true; } + /// Gets the currently stored value as an eina.Error. + public bool Get(out eina.Error value) + { + SanityChecks(); + bool ret; + int intermediate; // It seems out doesn't play well with implicit operators... + if (this.Optional) + ret = eina_value_optional_pget(this.Handle, out intermediate); + else + ret = eina_value_get_wrapper(this.Handle, out intermediate); + + value = intermediate; + + return ret; + } + /// Gets the currently stored value as an complex (e.g. container) eina.Value. public bool Get(out Value value) { diff --git a/src/lib/efl_mono/efl_custom_exports_mono.c b/src/lib/efl_mono/efl_custom_exports_mono.c index ac38ac7fa9..246a7bcf71 100644 --- a/src/lib/efl_mono/efl_custom_exports_mono.c +++ b/src/lib/efl_mono/efl_custom_exports_mono.c @@ -27,6 +27,11 @@ EAPI void *efl_mono_native_alloc(unsigned int size) return malloc(size); } +EAPI void efl_mono_native_memset(void *ptr, unsigned int fill, unsigned int count) +{ + memset(ptr, fill, count); +} + EAPI void efl_mono_native_free(void *ptr) { free(ptr); @@ -313,6 +318,9 @@ EAPI const Eina_Value_Type *type_array() { EAPI const Eina_Value_Type *type_list() { return EINA_VALUE_TYPE_LIST; } +EAPI const Eina_Value_Type *type_error() { + return EINA_VALUE_TYPE_ERROR; +} EAPI const Eina_Value_Type *type_optional() { return EINA_VALUE_TYPE_OPTIONAL; @@ -354,6 +362,11 @@ EAPI void eina_value_flush_wrapper(Eina_Value *value) EAPI const Eina_Value_Type *eina_value_type_get_wrapper(const Eina_Value *value) { + EINA_SAFETY_ON_NULL_RETURN_VAL(value, NULL); + + // Can't pass null value type (for Empty values) to value_type_get. + if (value->type == NULL) + return NULL; return eina_value_type_get(value); } diff --git a/src/tests/efl_mono/Value.cs b/src/tests/efl_mono/Value.cs index 018019b151..78bf0b4b50 100644 --- a/src/tests/efl_mono/Value.cs +++ b/src/tests/efl_mono/Value.cs @@ -135,6 +135,17 @@ public static class TestEinaValue { } } + public static void TestErrorSimple() + { + using (eina.Value v = new eina.Value(eina.ValueType.Error)) { + eina.Error error = new eina.Error(eina.Error.NO_ERROR); + Test.Assert(v.Set(error)); + eina.Error x; + Test.Assert(v.Get(out x)); + Test.AssertEquals(error, x); + } + } + public static void TestSetWrongType() { using (eina.Value v = new eina.Value(eina.ValueType.String)) { @@ -209,20 +220,20 @@ public static class TestEinaValue { { using (eina.Value a = new eina.Value(eina.ValueType.Optional)) { Test.Assert(a.Optional); - Test.Assert(a.Empty); // By default, optional values are empty + Test.Assert(a.OptionalEmpty); // By default, optional values are empty // Sets expectation int expected = 1984; Test.Assert(a.Set(expected)); Test.Assert(a.Optional); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); Test.Assert(a.Reset()); - Test.Assert(a.Empty); + Test.Assert(a.OptionalEmpty); expected = -4891; Test.Assert(a.Set(expected)); // Set() automatically infers the subtype from the argument. - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); int actual = 0; Test.Assert(a.Get(out actual)); @@ -233,20 +244,20 @@ public static class TestEinaValue { { using (eina.Value a = new eina.Value(eina.ValueType.Optional)) { Test.Assert(a.Optional); - Test.Assert(a.Empty); // By default, optional values are empty + Test.Assert(a.OptionalEmpty); // By default, optional values are empty // Sets expectation uint expected = 1984; Test.Assert(a.Set(expected)); Test.Assert(a.Optional); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); Test.Assert(a.Reset()); - Test.Assert(a.Empty); + Test.Assert(a.OptionalEmpty); expected = 0xdeadbeef; Test.Assert(a.Set(expected)); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); uint actual = 0; Test.Assert(a.Get(out actual)); @@ -257,26 +268,26 @@ public static class TestEinaValue { { using (eina.Value a = new eina.Value(eina.ValueType.Int32)) { Test.Assert(!a.Optional); - BoolRet dummy = () => a.Empty; + BoolRet dummy = () => a.OptionalEmpty; Test.AssertRaises(() => dummy()); } using (eina.Value a = new eina.Value(eina.ValueType.Optional)) { Test.Assert(a.Optional); - Test.Assert(a.Empty); // By default, optional values are empty + Test.Assert(a.OptionalEmpty); // By default, optional values are empty // Sets expectation string expected = "Hello, world!"; Test.Assert(a.Set(expected)); Test.Assert(a.Optional); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); Test.Assert(a.Reset()); - Test.Assert(a.Empty); + Test.Assert(a.OptionalEmpty); expected = "!dlrow olleH"; Test.Assert(a.Set(expected)); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); string actual = String.Empty; Test.Assert(a.Get(out actual)); @@ -291,7 +302,7 @@ public static class TestEinaValue { { Test.Assert(a.Optional); - Test.Assert(a.Empty); // By default, optional values are empty + Test.Assert(a.OptionalEmpty); // By default, optional values are empty // Sets expectation Test.Assert(expected.Append(-1)); @@ -300,14 +311,14 @@ public static class TestEinaValue { Test.Assert(a.Set(expected)); Test.Assert(a.Optional); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); Test.Assert(a.Reset()); - Test.Assert(a.Empty); + Test.Assert(a.OptionalEmpty); expected.Append(-42); Test.Assert(a.Set(expected)); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); eina.Value actual = null; Test.Assert(a.Get(out actual)); @@ -325,7 +336,7 @@ public static class TestEinaValue { { Test.Assert(a.Optional); - Test.Assert(a.Empty); // By default, optional values are empty + Test.Assert(a.OptionalEmpty); // By default, optional values are empty // Sets expectation Test.Assert(expected.Append(-1)); @@ -334,14 +345,14 @@ public static class TestEinaValue { Test.Assert(a.Set(expected)); Test.Assert(a.Optional); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); Test.Assert(a.Reset()); - Test.Assert(a.Empty); + Test.Assert(a.OptionalEmpty); expected.Append(-42); Test.Assert(a.Set(expected)); - Test.Assert(!a.Empty); + Test.Assert(!a.OptionalEmpty); eina.Value actual = null; Test.Assert(a.Get(out actual)); @@ -807,9 +818,23 @@ public static class TestEinaValue { value_ptr.Set(payload); eina.Value_Native byvalue = value_ptr; eina.Value another_value_ptr = byvalue; + Test.AssertEquals(value_ptr, another_value_ptr); } } + public static void TestValueEmpty() { + using (eina.Value empty = new eina.Value(eina.ValueType.Empty)) { + Test.Assert(empty.Empty, "Value must be empty"); + + empty.Setup(eina.ValueType.Int32); + + // Values already set-up are not empty. For this kind of empty, use Optional + Test.Assert(!empty.Empty, "Values already set-up must not be empty."); + + empty.Set(42); + Test.Assert(!empty.Empty, "Values with payload must not be empty."); + } + } // FIXME Add remaining list tests diff --git a/src/tests/efl_mono/ValueEolian.cs b/src/tests/efl_mono/ValueEolian.cs index 3f39004e54..4c3531544b 100644 --- a/src/tests/efl_mono/ValueEolian.cs +++ b/src/tests/efl_mono/ValueEolian.cs @@ -77,23 +77,6 @@ public static class TestEinaValueEolian { } } - public static void TestEolianEinaValueInReturnByValue() - { - test.ITesting obj = new test.Testing(); - - using (eina.Value v = new eina.Value(eina.ValueType.Int32)) { - v.Set(42); - obj.SetValue(v); - Test.AssertEquals(eina.Ownership.Managed, v.Ownership); - - // Using get_value_ptr while get_value() is not supported. - eina.Value v_received = obj.GetValuePtrOwn(); - Test.AssertEquals(eina.Ownership.Managed, v_received.Ownership); - Test.AssertEquals(v, v_received); - v_received.Dispose(); - } - } - public static void TestEolianEinaValueOutByValue() { test.ITesting obj = new test.Testing(); diff --git a/src/tests/efl_mono/libefl_mono_native_test.c b/src/tests/efl_mono/libefl_mono_native_test.c index a20f1cdc89..1c8f0885d5 100644 --- a/src/tests/efl_mono/libefl_mono_native_test.c +++ b/src/tests/efl_mono/libefl_mono_native_test.c @@ -3680,14 +3680,6 @@ Eina_Value *_test_testing_get_value_ptr(EINA_UNUSED Eo *obj, Test_Testing_Data * return pd->stored_value; } -/* Currently the Eolian declaration FUNC_BODY in the .eo.c file seems to be broken for - * generic value. - */ -/* Eina_Value _test_testing_get_value(EINA_UNUSED Eo *obj, Test_Testing_Data *pd) -{ - return *pd->stored_value; -}*/ - void _test_testing_clear_value(EINA_UNUSED Eo *obj, Test_Testing_Data *pd) { if (pd->stored_value) {