On 24 October 2016 at 16:45, Jean-Philippe André <j...@videolan.org> wrote:

>
>
> On 24 October 2016 at 16:11, <marcel-hollerb...@t-online.de> wrote:
>
>> 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. :)
>>
>
> 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.



> 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 <
>> > > 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
>>
>>
>
>
> --
> 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
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to