From 56a74238d1fd475546373f2e2eddbd7ef55dd3fa Mon Sep 17 00:00:00 2001 From: Jean-Philippe Andre Date: Thu, 22 Jun 2017 09:05:41 +0900 Subject: [PATCH] edje: Improve error report with efl_part misuse This improves a rare error message when a function is called on an efl_part() that does not implement it. Example: calling a swallow function on a non-swallow part. This isn't entirely fool-proof but should already help quite a bit. This also changes how the efl_part proxies are stored: the variable is not reset to NULL every time we use it, instead we check it in the del intercept. Note: _part_reuse_error() can not be enabled inside _internal_proxy_get because there are valid use cases such as: func1(efl_part(obj, part), func2(efl_part(obj, part), ...), ...) Here we use two efl_part() at the same time, on the same object, but we haven't entered "func1" yet when we are reaching the second call to efl_part(). This is completely valid and there is pretty much no way to detect this. I think I will improve this later with a core function on Efl.Object like "debug_string". Ref T5584 --- src/lib/edje/edje_part.c | 76 ++++++++++++++++++++++++++++-- src/lib/edje/edje_part_box.c | 2 +- src/lib/edje/edje_part_external.c | 2 +- src/lib/edje/edje_part_helper.h | 98 ++++++++++++++++++++++----------------- src/lib/edje/edje_part_swallow.c | 2 +- src/lib/edje/edje_part_table.c | 2 +- src/lib/edje/edje_part_text.c | 2 +- 7 files changed, 131 insertions(+), 53 deletions(-) diff --git a/src/lib/edje/edje_part.c b/src/lib/edje/edje_part.c index cbfd71e..eb09b93 100644 --- a/src/lib/edje/edje_part.c +++ b/src/lib/edje/edje_part.c @@ -2,9 +2,55 @@ #include "edje_part_helper.h" #define MY_CLASS EFL_CANVAS_LAYOUT_INTERNAL_CLASS -PROXY_IMPLEMENTATION(other, INTERNAL, EINA_FALSE) +PROXY_IMPLEMENTATION(other, MY_CLASS, EINA_FALSE) #undef PROXY_IMPLEMENTATION +void +_part_reuse_error(Efl_Canvas_Layout_Internal_Data *pd) +{ + const char *typestr = "UNKNOWN"; + Edje *ed; + + // TODO: Enable full debug only for eo_debug? + // Don't trust pd->ed as it may be invalid now. + ed = efl_data_scope_safe_get(pd->obj, EDJE_OBJECT_CLASS); + if (!ed) + { + ERR("A previous misuse of efl_part has been detected. Handles returned " + "by efl_part() are valid for a single function call. Did you call " + "a non implemented function? obj: %p has been deleted!", pd->obj); + return; + } + + switch (pd->rp->part->type) + { + case EDJE_PART_TYPE_RECTANGLE: typestr = "RECTANGLE"; break; + case EDJE_PART_TYPE_TEXT: typestr = "TEXT"; break; + case EDJE_PART_TYPE_IMAGE: typestr = "IMAGE"; break; + case EDJE_PART_TYPE_SWALLOW: typestr = "SWALLOW"; break; + case EDJE_PART_TYPE_TEXTBLOCK: typestr = "TEXTBLOCK"; break; + case EDJE_PART_TYPE_GRADIENT: typestr = "GRADIENT"; break; + case EDJE_PART_TYPE_GROUP: typestr = "GROUP"; break; + case EDJE_PART_TYPE_BOX: typestr = "BOX"; break; + case EDJE_PART_TYPE_TABLE: typestr = "TABLE"; break; + case EDJE_PART_TYPE_EXTERNAL: typestr = "EXTERNAL"; break; + case EDJE_PART_TYPE_PROXY: typestr = "PROXY"; break; + case EDJE_PART_TYPE_SPACER: typestr = "SPACER"; break; + case EDJE_PART_TYPE_MESH_NODE: typestr = "MESH_NODE"; break; + case EDJE_PART_TYPE_LIGHT: typestr = "LIGHT"; break; + case EDJE_PART_TYPE_CAMERA: typestr = "CAMERA"; break; + case EDJE_PART_TYPE_SNAPSHOT: typestr = "SNAPSHOT"; break; + case EDJE_PART_TYPE_VECTOR: typestr = "VECTOR"; break; + default: break; + } + + ERR("A previous misuse of efl_part has been detected. Handles returned " + "by efl_part() are valid for a single function call. Did you call " + "a non implemented function? obj: %p group: '%s' part: '%s' type: %s%s", + pd->obj, ed->group, pd->part, typestr, + ed->delete_me ? ". This object is already deleted." : ""); +} + PROXY_INIT(box) PROXY_INIT(table) PROXY_INIT(swallow) @@ -24,20 +70,25 @@ _edje_internal_proxy_shutdown(void) } void -_edje_real_part_set(Eo *obj, void *ed, void *rp, const char *part) +_edje_real_part_set(Eo *obj, Edje *ed, Edje_Real_Part *rp, const char *part) { - PROXY_DATA_GET(obj, pd); + Efl_Canvas_Layout_Internal_Data *pd; + + pd = efl_data_scope_get(obj, EFL_CANVAS_LAYOUT_INTERNAL_CLASS); + pd->obj = ed->obj; pd->ed = ed; pd->rp = rp; pd->part = part; pd->temp = 1; - efl_parent_set(obj, pd->ed->obj); + pd->in_call = 0; + efl_parent_set(obj, ed->obj); } EOLIAN static Efl_Object * _efl_canvas_layout_internal_efl_object_finalize(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd) { EINA_SAFETY_ON_FALSE_RETURN_VAL(pd->rp && pd->ed && pd->part, NULL); + // Do not use RETURN_VAL() here! return efl_finalize(efl_super(obj, MY_CLASS)); } @@ -45,6 +96,7 @@ EOLIAN void _efl_canvas_layout_internal_efl_gfx_geometry_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, int *x, int *y, int *w, int *h) { Edje_Real_Part *rp = pd->rp; + PROXY_CALL_BEGIN(pd); _edje_recalc_do(pd->ed); if (!rp) @@ -53,6 +105,7 @@ _efl_canvas_layout_internal_efl_gfx_geometry_get(Eo *obj, Efl_Canvas_Layout_Inte if (y) *y = 0; if (w) *w = 0; if (h) *h = 0; + RETURN_VOID; } if (x) *x = rp->x; @@ -66,8 +119,10 @@ EOLIAN static void _efl_canvas_layout_internal_state_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, const char **name, double *val) { const char *str; + PROXY_CALL_BEGIN(pd); - if (!name && !val) return; + if (!name && !val) + RETURN_VOID; str = _edje_object_part_state_get(pd->ed, pd->part, val); if (name) *name = str; RETURN_VOID; @@ -76,66 +131,77 @@ _efl_canvas_layout_internal_state_get(Eo *obj, Efl_Canvas_Layout_Internal_Data * EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_value_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_value_set(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_value_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dx, double *dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_value_get(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_size_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dw, double dh) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_size_set(pd->ed, pd->part, dw, dh)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_size_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dw, double *dh) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_size_get(pd->ed, pd->part, dw, dh)); } EOLIAN static Efl_Ui_Drag_Dir _efl_canvas_layout_internal_efl_ui_drag_drag_dir_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_dir_get(pd->ed, pd->part)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_step_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_step_set(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_step_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dx, double *dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_step_get(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_step_move(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_step(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_page_set(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_page_set(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_page_get(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double *dx, double *dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_page_get(pd->ed, pd->part, dx, dy)); } EOLIAN static Eina_Bool _efl_canvas_layout_internal_efl_ui_drag_drag_page_move(Eo *obj, Efl_Canvas_Layout_Internal_Data *pd, double dx, double dy) { + PROXY_CALL_BEGIN(pd); RETURN_VAL(_edje_object_part_drag_page(pd->ed, pd->part, dx, dy)); } diff --git a/src/lib/edje/edje_part_box.c b/src/lib/edje/edje_part_box.c index f2ae4b6..6e537fe 100644 --- a/src/lib/edje/edje_part_box.c +++ b/src/lib/edje/edje_part_box.c @@ -5,7 +5,7 @@ #include "../evas/canvas/evas_box.eo.h" -PROXY_IMPLEMENTATION(box, INTERNAL_BOX, EINA_FALSE) +PROXY_IMPLEMENTATION(box, MY_CLASS, EINA_FALSE) #undef PROXY_IMPLEMENTATION typedef struct _Part_Item_Iterator Part_Item_Iterator; diff --git a/src/lib/edje/edje_part_external.c b/src/lib/edje/edje_part_external.c index 156d13f..1ffc416 100644 --- a/src/lib/edje/edje_part_external.c +++ b/src/lib/edje/edje_part_external.c @@ -5,7 +5,7 @@ static void _external_compose(Eo *obj, Edje *ed, const char *part); -PROXY_IMPLEMENTATION(external, EXTERNAL, EINA_TRUE, _external_compose(proxy, ed, rp->part->name)) +PROXY_IMPLEMENTATION(external, MY_CLASS, EINA_TRUE, _external_compose(proxy, ed, rp->part->name)) #undef PROXY_IMPLEMENTATION static void diff --git a/src/lib/edje/edje_part_helper.h b/src/lib/edje/edje_part_helper.h index 4ae1ae0..90f0185 100644 --- a/src/lib/edje/edje_part_helper.h +++ b/src/lib/edje/edje_part_helper.h @@ -8,7 +8,8 @@ struct _Efl_Canvas_Layout_Internal_Data Edje *ed; Edje_Real_Part *rp; const char *part; - unsigned char temp; + Eo *obj; + unsigned char temp, in_call; }; struct _Part_Item_Iterator @@ -19,26 +20,53 @@ struct _Part_Item_Iterator Eo *object; }; +void _part_reuse_error(Efl_Canvas_Layout_Internal_Data *pd); + +#define PROXY_CALL_BEGIN(pd) do { pd->in_call = 1; } while (0) +#define PROXY_CALL_END(pd) do { pd->in_call = 0; } while (0) #define PROXY_REF(obj, pd) do { if (!(pd->temp++)) efl_ref(obj); } while(0) #define PROXY_UNREF(obj, pd) do { if (pd->temp) { if (!(--pd->temp)) efl_del(obj); } } while(0) -#define RETURN_VAL(a) do { typeof(a) _ret = a; PROXY_UNREF(obj, pd); return _ret; } while(0) -#define RETURN_VOID do { PROXY_UNREF(obj, pd); return; } while(0) +#define RETURN_VAL(a) do { typeof(a) _ret = a; PROXY_CALL_END(pd); PROXY_UNREF(obj, pd); return _ret; } while(0) +#define RETURN_VOID do { PROXY_CALL_END(pd); PROXY_UNREF(obj, pd); return; } while(0) #define PROXY_CALL(a) ({ PROXY_REF(obj, pd); a; }) +#define PROXY_STATIC_VAR(type) _##type##_proxy #ifndef PROXY_ADD_EXTRA_OP # define PROXY_ADD_EXTRA_OP #endif -void _edje_real_part_set(Eo *obj, void *ed, void *rp, const char *part); +void _edje_real_part_set(Eo *obj, Edje *ed, Edje_Real_Part *rp, const char *part); + +static inline void +_part_proxy_del_cb(Eo *proxy, Eo **static_var) +{ + if (*static_var) + { + // FIXME: Enable debug checks only in debug mode + Efl_Canvas_Layout_Internal_Data *pd = efl_data_scope_get + (*static_var, EFL_CANVAS_LAYOUT_INTERNAL_CLASS); + if (pd && pd->temp && !pd->in_call) + _part_reuse_error(pd); + if (*static_var != proxy) + efl_del_intercept_set(*static_var, NULL); + } + if (efl_parent_get(proxy)) + { + efl_ref(proxy); + efl_parent_set(proxy, NULL); + } + efl_reuse(proxy); + *static_var = proxy; +} /* ugly macros to avoid code duplication */ #define PROXY_RESET(type) \ - do { if (_ ## type ## _proxy) \ + do { if (PROXY_STATIC_VAR(type)) \ { \ - efl_del_intercept_set(_ ## type ## _proxy, NULL); \ - efl_del(_ ## type ## _proxy); \ - _ ## type ## _proxy = NULL; \ + efl_del_intercept_set(PROXY_STATIC_VAR(type), NULL); \ + efl_del(PROXY_STATIC_VAR(type)); \ + PROXY_STATIC_VAR(type) = NULL; \ } } while (0) #define PROXY_INIT(type) \ @@ -46,27 +74,16 @@ void \ _ ## type ## _shutdown(void); \ #define PROXY_DATA_GET(obj, pd) \ - Efl_Canvas_Layout_Internal_Data *pd = efl_data_scope_get(obj, EFL_CANVAS_LAYOUT_INTERNAL_CLASS) + Efl_Canvas_Layout_Internal_Data *pd = efl_data_scope_get(obj, EFL_CANVAS_LAYOUT_INTERNAL_CLASS); \ + PROXY_CALL_BEGIN(pd) -#define PROXY_IMPLEMENTATION(type, TYPE, no_del_cb, ...) \ -static Eo * _ ## type ## _proxy = NULL; \ +#define PROXY_IMPLEMENTATION(type, KLASS, no_del_cb, ...) \ +static Eo * PROXY_STATIC_VAR(type) = NULL; \ \ static void \ _ ## type ## _del_cb(Eo *proxy) \ { \ - if (_ ## type ## _proxy) \ - { \ - efl_del_intercept_set(proxy, NULL); \ - efl_del(proxy); \ - return; \ - } \ - if (efl_parent_get(proxy)) \ - { \ - efl_ref(proxy); \ - efl_parent_set(proxy, NULL); \ - } \ - efl_reuse(proxy); \ - _ ## type ## _proxy = proxy; \ + _part_proxy_del_cb(proxy, &(PROXY_STATIC_VAR(type))); \ } \ \ void \ @@ -81,32 +98,27 @@ _edje_ ## type ## _internal_proxy_get(Edje_Object *obj EINA_UNUSED, Edje *ed, Ed Efl_Canvas_Layout_Internal_Data *pd; \ Eo *proxy; \ \ - pd = efl_data_scope_get(_ ## type ## _proxy, EFL_CANVAS_LAYOUT_INTERNAL_CLASS); \ + pd = PROXY_STATIC_VAR(type) ? efl_data_scope_get(PROXY_STATIC_VAR(type), EFL_CANVAS_LAYOUT_INTERNAL_CLASS) : NULL; \ if (!pd) \ { \ - if (_ ## type ## _proxy) \ - { \ - ERR("Found invalid handle for efl_part. Reset."); \ - _ ## type ## _proxy = NULL; \ - } \ - proxy = efl_add(EFL_CANVAS_LAYOUT_##TYPE##_CLASS, ed->obj, \ - _edje_real_part_set(efl_added, ed, rp, rp->part->name)); \ - __VA_ARGS__; \ - if (!no_del_cb) efl_del_intercept_set(proxy, _ ## type ## _del_cb); \ - return proxy; \ + if (PROXY_STATIC_VAR(type)) \ + ERR("Found invalid handle for efl_part. Reset."); \ + PROXY_STATIC_VAR(type) = efl_add(KLASS, ed->obj, _edje_real_part_set(efl_added, ed, rp, rp->part->name)); \ + goto end ; \ } \ \ if (EINA_UNLIKELY(pd->temp)) \ { \ - /* warn about misuse, since non-implemented functions may trigger this \ - * misuse by accident. */ \ - ERR("Misuse of efl_part detected. Handles returned by efl_part() are " \ - "valid for a single function call! Did you call a non implemented " \ - "function?"); \ + /* if (!pd->in_call) _part_reuse_error(pd); */ \ + PROXY_STATIC_VAR(type) = efl_add(KLASS, ed->obj, _edje_real_part_set(efl_added, ed, rp, rp->part->name)); \ } \ - proxy = _ ## type ## _proxy; \ - _ ## type ## _proxy = NULL; \ - _edje_real_part_set(proxy, ed, rp, rp->part->name); \ + else \ + { \ + _edje_real_part_set(PROXY_STATIC_VAR(type), ed, rp, rp->part->name); \ + } \ + \ +end: \ + proxy = PROXY_STATIC_VAR(type); \ __VA_ARGS__; \ if (!no_del_cb) efl_del_intercept_set(proxy, _ ## type ## _del_cb); \ return proxy; \ diff --git a/src/lib/edje/edje_part_swallow.c b/src/lib/edje/edje_part_swallow.c index a85fdb9..4e9564d 100644 --- a/src/lib/edje/edje_part_swallow.c +++ b/src/lib/edje/edje_part_swallow.c @@ -3,7 +3,7 @@ #include "efl_canvas_layout_internal_swallow.eo.h" #define MY_CLASS EFL_CANVAS_LAYOUT_INTERNAL_SWALLOW_CLASS -PROXY_IMPLEMENTATION(swallow, INTERNAL_SWALLOW, EINA_FALSE) +PROXY_IMPLEMENTATION(swallow, MY_CLASS, EINA_FALSE) #undef PROXY_IMPLEMENTATION /* Swallow parts */ diff --git a/src/lib/edje/edje_part_table.c b/src/lib/edje/edje_part_table.c index c1bc0b4..211dce1 100644 --- a/src/lib/edje/edje_part_table.c +++ b/src/lib/edje/edje_part_table.c @@ -5,7 +5,7 @@ #include "../evas/canvas/evas_table.eo.h" -PROXY_IMPLEMENTATION(table, INTERNAL_TABLE, EINA_FALSE) +PROXY_IMPLEMENTATION(table, MY_CLASS, EINA_FALSE) #undef PROXY_IMPLEMENTATION typedef struct _Part_Item_Iterator Part_Item_Iterator; diff --git a/src/lib/edje/edje_part_text.c b/src/lib/edje/edje_part_text.c index 027863a..bc848fc 100644 --- a/src/lib/edje/edje_part_text.c +++ b/src/lib/edje/edje_part_text.c @@ -3,7 +3,7 @@ #include "efl_canvas_layout_internal_text.eo.h" #define MY_CLASS EFL_CANVAS_LAYOUT_INTERNAL_TEXT_CLASS -PROXY_IMPLEMENTATION(text, INTERNAL_TEXT, EINA_FALSE) +PROXY_IMPLEMENTATION(text, MY_CLASS, EINA_FALSE) #undef PROXY_IMPLEMENTATION EOLIAN static void -- 2.7.4