On Mon, Oct 24, 2016 at 12:21:19PM +0900, Jean-Philippe André wrote:
> Hey Marcel,
> 
> On 23 October 2016 at 16:45, <marcel-hollerb...@t-online.de> 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. :)

> 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 <
> > marcel-hollerb...@t-online.de>
> > > :
> > >
> > > > bu5hm4n pushed a commit to branch master.
> > > >
> > > > http://git.enlightenment.org/core/efl.git/commit/?id=
> > > > 0180da708dda0d95fc34ec68c7d65d2df9ab4f95
> > > >
> > > > commit 0180da708dda0d95fc34ec68c7d65d2df9ab4f95
> > > > Author: Marcel Hollerbach <marcel-hollerb...@t-online.de>
> > > > 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
> > > enlightenment-devel@lists.sourceforge.net
> > > 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
> > enlightenment-devel@lists.sourceforge.net
> > 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
> enlightenment-devel@lists.sourceforge.net
> 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
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to