On Mon, 24 Oct 2016 17:02:54 +0900 Jean-Philippe André <[email protected]> said:
> On 24 October 2016 at 16:45, Jean-Philippe André <[email protected]> wrote: > > > > > > > On 24 October 2016 at 16:11, <[email protected]> wrote: > > > >> On Mon, Oct 24, 2016 at 12:21:19PM +0900, Jean-Philippe André wrote: > >> > Hey Marcel, > >> > > >> > On 23 October 2016 at 16:45, <[email protected]> wrote: > >> > > >> > > I am not wondering thats its YOU dont who dont like my change... :) > >> > > > >> > > Well there are two reasons to NOT have a Error message. > >> > > > >> > > First, it was before like that, so before it was completly valid, no > >> > > error, now you will have a shitload of error messages in your console. > >> > > Which was the case since evas_object_event_callback_del_full was > >> called > >> > > on cursor objects, undependent from if they are NULL or not. So if you > >> > > dont want to have this NULL check, feel free the go to all api calls > >> in > >> > > EFL and check if they are valid. > >> > > > >> > > Also, complete eo works like that, if you call things on a NULL > >> object, > >> > > nothing will happen ... So i guess the same for evas_object_* > >> functions > >> > > should be the case. > >> > > > >> > > So no, its a correct usecase. > >> > > > >> > > >> > Not really. > >> > It's not because it was silently ignored that the calls were actually > >> valid. > >> > > >> > In fact doing something on NULL (except del) should trigger an error or > >> > warning message. > >> > Hiding error messages is really just that: it's hiding an invalid code > >> path. > >> > >> This does not make sense. > >> > >> Honestly, why should: > >> > >> ecore_event_handler_del(NULL) > >> > >> be fine but: > >> > >> evas_object_event_callback_del_full(NULL) > >> > >> give a error ? > >> > >> Even more, all the functions from eo, just dont do anything, why > >> should this single function give a error? Sure its usefull to get > >> warned, HEY you are doing this on a null obj, but if we do so it should > >> be consistent over all the api, and not just those few evas api´s. > >> But i guess this will not fit into anyones time scheudle. :) > >> > > > > If you call an EO function on a NULL object you get an error message (a > > cryptic one btw, should be improved). > > > > ^^ I was wrong, didn't compile properly :) (now the coffee has finally > kicked in) > And yes, adding ERR on NULL calls triggers a ton of ERR messages. > > My bad. I get your point then. which is why this should be DBG(). :) leave ERR for real errors. libc doesnt complain or segv when you free(NULL). so by the same token i kind of see efl_func(NULL...) where thats the obj is like free(NULL) - its a nop op. maybe you didnt intend it... but we shouldnt be hellishly noisy all day about it. all that happens is code getys littered with if (obj) api(obj); we already have a tonne of it and all it does is make code slower and bigger and longer. > > So the above scenario where you call a function that silently does nothing > > on NULL is not going to happen much as we move to EO. > > > > One way or another remember that those are only debug logs. Not a > > functional change. > > More debug is usually better, maybe ERR is too high a level. But ignoring > > errors silently is a sure way to add bugs in the future :) > > > > As you said on IRC, this could be a feature of eo_dbg. > > Best regards, > jp > > > > > > > > > Let's instead fix the root causes for those calls on NULL. > >> > Yes, that more work, and it also shows more error logs in existing > >> careless > >> > apps. In the long run, this helps. > >> > > >> > Best regards, > >> > > >> > > >> > On Sun, Oct 23, 2016 at 08:56:10AM +0200, Davide Andreoli wrote: > >> > > > hey, I don't like this change. passing a NULL obj to those functions > >> > > can't > >> > > > be correct and an error message is imo the right think to do. > >> > > > In fact yesterday I spotted 2 hidden bugs in python-efl thanks to > >> the > >> > > > message you removed. > >> > > > Do you have a correct use case for this? or you are just lazy and > >> don't > >> > > > want to see errors on console? :P > >> > > > > >> > > > 2016-10-22 20:32 GMT+02:00 Marcel Hollerbach < > >> > > [email protected]> > >> > > > : > >> > > > > >> > > > > bu5hm4n pushed a commit to branch master. > >> > > > > > >> > > > > http://git.enlightenment.org/core/efl.git/commit/?id= > >> > > > > 0180da708dda0d95fc34ec68c7d65d2df9ab4f95 > >> > > > > > >> > > > > commit 0180da708dda0d95fc34ec68c7d65d2df9ab4f95 > >> > > > > Author: Marcel Hollerbach <[email protected]> > >> > > > > Date: Sat Oct 22 19:26:47 2016 +0200 > >> > > > > > >> > > > > evas_callbacks: restore error message behaviour from > >> MAGIC_CHECK > >> > > > > > >> > > > > before changing MAGIC_CHECK to eo_isa passing NULL to a > >> function > >> > > would > >> > > > > result in nothing, now it gives a error message. This > >> restores the > >> > > old > >> > > > > behaviour. > >> > > > > --- > >> > > > > src/lib/evas/canvas/evas_callbacks.c | 9 +++++++++ > >> > > > > 1 file changed, 9 insertions(+) > >> > > > > > >> > > > > diff --git a/src/lib/evas/canvas/evas_callbacks.c > >> > > > > b/src/lib/evas/canvas/evas_callbacks.c > >> > > > > index 7842e7c..18117bf 100644 > >> > > > > --- a/src/lib/evas/canvas/evas_callbacks.c > >> > > > > +++ b/src/lib/evas/canvas/evas_callbacks.c > >> > > > > @@ -386,6 +386,7 @@ evas_object_event_callback_add(Evas_Object > >> > > *eo_obj, > >> > > > > Evas_Callback_Type type, Eva > >> > > > > EAPI void > >> > > > > evas_object_event_callback_priority_add(Evas_Object *eo_obj, > >> > > > > Evas_Callback_Type type, Evas_Callback_Priority priority, > >> > > > > Evas_Object_Event_Cb func, const void *data) > >> > > > > { > >> > > > > + if(!eo_obj) return; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN(efl_isa(eo_obj, > >> > > EFL_CANVAS_OBJECT_CLASS)); > >> > > > > Evas_Object_Protected_Data *obj = efl_data_scope_get(eo_obj, > >> > > > > EFL_CANVAS_OBJECT_CLASS); > >> > > > > > >> > > > > @@ -408,6 +409,7 @@ evas_object_event_callback_ > >> > > priority_add(Evas_Object > >> > > > > *eo_obj, Evas_Callback_Type > >> > > > > EAPI void * > >> > > > > evas_object_event_callback_del(Evas_Object *eo_obj, > >> > > Evas_Callback_Type > >> > > > > type, Evas_Object_Event_Cb func) > >> > > > > { > >> > > > > + if(!eo_obj) return NULL; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN_VAL(efl_isa(eo_obj, > >> > > > > EFL_CANVAS_OBJECT_CLASS), NULL); > >> > > > > Evas_Object_Protected_Data *obj = efl_data_scope_get(eo_obj, > >> > > > > EFL_CANVAS_OBJECT_CLASS); > >> > > > > _eo_evas_object_cb_info *info; > >> > > > > @@ -436,6 +438,7 @@ evas_object_event_callback_del(Evas_Object > >> > > *eo_obj, > >> > > > > Evas_Callback_Type type, Eva > >> > > > > EAPI void * > >> > > > > evas_object_event_callback_del_full(Evas_Object *eo_obj, > >> > > > > Evas_Callback_Type type, Evas_Object_Event_Cb func, const void > >> *data) > >> > > > > { > >> > > > > + if(!eo_obj) return NULL; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN_VAL(efl_isa(eo_obj, > >> > > > > EFL_CANVAS_OBJECT_CLASS), NULL); > >> > > > > Evas_Object_Protected_Data *obj = efl_data_scope_get(eo_obj, > >> > > > > EFL_CANVAS_OBJECT_CLASS); > >> > > > > _eo_evas_object_cb_info *info; > >> > > > > @@ -471,6 +474,7 @@ evas_event_callback_add(Evas *eo_e, > >> > > Evas_Callback_Type > >> > > > > type, Evas_Event_Cb func, > >> > > > > EAPI void > >> > > > > evas_event_callback_priority_add(Evas *eo_e, Evas_Callback_Type > >> type, > >> > > > > Evas_Callback_Priority priority, Evas_Event_Cb func, const void > >> *data) > >> > > > > { > >> > > > > + if(!eo_e) return; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN(efl_isa(eo_e, > >> EVAS_CANVAS_CLASS)); > >> > > > > Evas_Public_Data *e = efl_data_scope_get(eo_e, > >> EVAS_CANVAS_CLASS); > >> > > > > > >> > > > > @@ -490,6 +494,7 @@ evas_event_callback_priority_add(Evas *eo_e, > >> > > > > Evas_Callback_Type type, Evas_Callb > >> > > > > EAPI void * > >> > > > > evas_event_callback_del(Evas *eo_e, Evas_Callback_Type type, > >> > > > > Evas_Event_Cb func) > >> > > > > { > >> > > > > + if(!eo_e) return NULL; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN_VAL(efl_isa(eo_e, > >> EVAS_CANVAS_CLASS), > >> > > > > NULL); > >> > > > > Evas_Public_Data *e = efl_data_scope_get(eo_e, > >> EVAS_CANVAS_CLASS); > >> > > > > _eo_evas_cb_info *info; > >> > > > > @@ -518,6 +523,7 @@ evas_event_callback_del(Evas *eo_e, > >> > > Evas_Callback_Type > >> > > > > type, Evas_Event_Cb func) > >> > > > > EAPI void * > >> > > > > evas_event_callback_del_full(Evas *eo_e, Evas_Callback_Type > >> type, > >> > > > > Evas_Event_Cb func, const void *data) > >> > > > > { > >> > > > > + if(!eo_e) return NULL; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN_VAL(efl_isa(eo_e, > >> EVAS_CANVAS_CLASS), > >> > > > > NULL); > >> > > > > Evas_Public_Data *e = efl_data_scope_get(eo_e, > >> EVAS_CANVAS_CLASS); > >> > > > > _eo_evas_cb_info *info; > >> > > > > @@ -546,6 +552,7 @@ evas_event_callback_del_full(Evas *eo_e, > >> > > > > Evas_Callback_Type type, Evas_Event_Cb > >> > > > > EAPI void > >> > > > > evas_post_event_callback_push(Evas *eo_e, > >> Evas_Object_Event_Post_Cb > >> > > > > func, const void *data) > >> > > > > { > >> > > > > + if(!eo_e) return; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN(efl_isa(eo_e, > >> EVAS_CANVAS_CLASS)); > >> > > > > Evas_Public_Data *e = efl_data_scope_get(eo_e, > >> EVAS_CANVAS_CLASS); > >> > > > > Evas_Post_Callback *pc; > >> > > > > @@ -565,6 +572,7 @@ evas_post_event_callback_push(Evas *eo_e, > >> > > > > Evas_Object_Event_Post_Cb func, const > >> > > > > EAPI void > >> > > > > evas_post_event_callback_remove(Evas *eo_e, > >> Evas_Object_Event_Post_Cb > >> > > > > func) > >> > > > > { > >> > > > > + if(!eo_e) return; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN(efl_isa(eo_e, > >> EVAS_CANVAS_CLASS)); > >> > > > > Evas_Public_Data *e = efl_data_scope_get(eo_e, > >> EVAS_CANVAS_CLASS); > >> > > > > Evas_Post_Callback *pc; > >> > > > > @@ -584,6 +592,7 @@ evas_post_event_callback_remove(Evas *eo_e, > >> > > > > Evas_Object_Event_Post_Cb func) > >> > > > > EAPI void > >> > > > > evas_post_event_callback_remove_full(Evas *eo_e, > >> > > > > Evas_Object_Event_Post_Cb func, const void *data) > >> > > > > { > >> > > > > + if(!eo_e) return; > >> > > > > EINA_SAFETY_ON_FALSE_RETURN(efl_isa(eo_e, > >> EVAS_CANVAS_CLASS)); > >> > > > > Evas_Public_Data *e = efl_data_scope_get(eo_e, > >> EVAS_CANVAS_CLASS); > >> > > > > Evas_Post_Callback *pc; > >> > > > > > >> > > > > -- > >> > > > > > >> > > > > > >> > > > > > >> > > > ------------------------------------------------------------ > >> > > ------------------ > >> > > > Check out the vibrant tech community on one of the world's most > >> > > > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > >> > > > _______________________________________________ > >> > > > enlightenment-devel mailing list > >> > > > [email protected] > >> > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > > > >> > > ------------------------------------------------------------ > >> > > ------------------ > >> > > Check out the vibrant tech community on one of the world's most > >> > > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > >> > > _______________________________________________ > >> > > enlightenment-devel mailing list > >> > > [email protected] > >> > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > > > >> > > > >> > > >> > > >> > -- > >> > Jean-Philippe André > >> > ------------------------------------------------------------ > >> ------------------ > >> > Check out the vibrant tech community on one of the world's most > >> > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > >> > _______________________________________________ > >> > enlightenment-devel mailing list > >> > [email protected] > >> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > >> ------------------------------------------------------------ > >> ------------------ > >> Check out the vibrant tech community on one of the world's most > >> engaging tech sites, SlashDot.org! http://sdm.link/slashdot > >> _______________________________________________ > >> enlightenment-devel mailing list > >> [email protected] > >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > >> > > > > > > -- > > Jean-Philippe André > > > > > > -- > Jean-Philippe André > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) [email protected] ------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
