On Thu, 25 Jun 2015 11:03:38 +0200 Cedric BAIL <[email protected]> said:

> On Thu, Jun 25, 2015 at 10:41 AM, Carsten Haitzler <[email protected]>
> wrote:
> > On Thu, 25 Jun 2015 09:27:34 +0200 Cedric BAIL <[email protected]> said:
> >> Le 25 juin 2015 06:19, "Carsten Haitzler" <[email protected]> a écrit :
> >> >
> >> > raster pushed a commit to branch master.
> >> >
> >> >
> >> http://git.enlightenment.org/core/efl.git/commit/?id=6483dc3ce91f94dd147bda1080b0e923d16f4653
> >> >
> >> > commit 6483dc3ce91f94dd147bda1080b0e923d16f4653
> >> > Author: Carsten Haitzler (Rasterman) <[email protected]>
> >> > Date:   Thu Jun 25 13:18:22 2015 +0900
> >> >
> >> >     ecore con - fix object data referencing for deleted objects
> >> >
> >> >     if the object has been deleted already, scope data is null. handle it
> >> >     correctly. this fixes a segv in the new efreetd when it starts and
> >> >     there is an existing efreet running thus owning the socket fails.
> >> >
> >> >     @fix
> >>
> >> You have been tricked by it to. Data scope get doesn't check anything and
> >> can return a valid pointer when asking the wrong class on an object. You
> >> usually should not check the return with null as it is hiding a bigger
> >> problem.
> >
> > the bigger problem is the deferred events etc. et.c and that an eo object
> > rewrite of ecore-con/ipc will fix. thus "this will do for now". :)
> 
> My point is that it can't do :-) eo_data_scope_get will either suceed
> or return gargabe, but != NULL. So testing if it is != NULL is just

nope. wrong. it returns NULL if the obj id is invalid. :)

> going to make you feel good and anyone looking at the code to which
> will make it harder to fix the real issue later on. Basically just
> remove all test != NULL related to the output of an eo_data_scope_get.
> That pattern is seriously misleading and far from obvious for C dev.

no. this is ACTUALLY a segfault. it's not a feel-good. :) it fixed it.

> >> > ---
> >> >  src/lib/ecore_con/ecore_con.c | 210
> >> +++++++++++++++++++++++++-----------------
> >> >  1 file changed, 124 insertions(+), 86 deletions(-)
> >> >
> >> > diff --git a/src/lib/ecore_con/ecore_con.c
> >> > b/src/lib/ecore_con/ecore_con.c index 941dc11..1582c26 100644
> >> > --- a/src/lib/ecore_con/ecore_con.c
> >> > +++ b/src/lib/ecore_con/ecore_con.c
> >> > @@ -271,6 +271,7 @@ ecore_con_shutdown(void)
> >> >          Ecore_Con_Server_Data *svr = eo_data_scope_get(obj,
> >> ECORE_CON_SERVER_CLASS);
> >> >          Ecore_Con_Event_Server_Add *ev;
> >> >
> >> > +        if (!svr) continue;
> >> >          svr->delete_me = EINA_TRUE;
> >> >          INF("svr %p is dead", svr);
> >> >          /* some pointer hacks here to prevent double frees if people are
> >> being stupid */
> >> > @@ -1373,8 +1374,10 @@ _ecore_con_server_eo_base_destructor(Eo *obj,
> >> Ecore_Con_Server_Data *svr)
> >> >
> >> >     ecore_con_ssl_server_shutdown(obj);
> >> >     free(svr->name);
> >> > +   svr->name = NULL;
> >> >
> >> >     free(svr->path);
> >> > +   svr->path = NULL;
> >> >
> >> >     eina_stringshare_del(svr->ip);
> >> >     eina_stringshare_del(svr->verify_name);
> >> > @@ -1394,8 +1397,9 @@ _ecore_con_server_eo_base_destructor(Eo *obj,
> >> Ecore_Con_Server_Data *svr)
> >> >     servers = eina_list_remove(servers, obj);
> >> >     svr->data = NULL;
> >> >
> >> > -end:
> >> >     eo_do_super(obj, ECORE_CON_SERVER_CLASS, eo_destructor());
> >> > +end:
> >> > +   return;
> >> >  }
> >> >
> >> >  static void
> >> > @@ -2675,21 +2679,25 @@ _ecore_con_event_client_add_free(Ecore_Con_Server
> >> *obj,
> >> >          Ecore_Con_Client_Data *cl = eo_data_scope_get(e->client,
> >> ECORE_CON_CLIENT_CLASS);
> >> >          Eina_Bool svrfreed = EINA_FALSE;
> >> >
> >> > -        cl->event_count = eina_list_remove(cl->event_count, e);
> >> > -        if (cl->host_server)
> >> > +        if ((svr) && (cl))
> >> >            {
> >> > -             Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > -             host_server->event_count =
> >> eina_list_remove(host_server->event_count, ev);
> >> > -             if ((!svr->event_count) && (svr->delete_me))
> >> > +             cl->event_count = eina_list_remove(cl->event_count, e);
> >> > +             if (cl->host_server)
> >> >                 {
> >> > -                  _ecore_con_server_free(obj);
> >> > -                  svrfreed = EINA_TRUE;
> >> > +                  Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > +                  if (host_server)
> >> > +                    host_server->event_count =
> >> eina_list_remove(host_server->event_count, ev);
> >> > +                  if ((!svr->event_count) && (svr->delete_me))
> >> > +                    {
> >> > +                       _ecore_con_server_free(obj);
> >> > +                       svrfreed = EINA_TRUE;
> >> > +                    }
> >> > +               }
> >> > +             if (!svrfreed)
> >> > +               {
> >> > +                  if ((!cl->event_count) && (cl->delete_me))
> >> > +                    ecore_con_client_del(e->client);
> >> >                 }
> >> > -          }
> >> > -        if (!svrfreed)
> >> > -          {
> >> > -             if ((!cl->event_count) && (cl->delete_me))
> >> > -               ecore_con_client_del(e->client);
> >> >            }
> >> >       }
> >> >
> >> > @@ -2712,21 +2720,25 @@ _ecore_con_event_client_del_free(Ecore_Con_Server
> >> *obj,
> >> >          Ecore_Con_Client_Data *cl = eo_data_scope_get(e->client,
> >> ECORE_CON_CLIENT_CLASS);
> >> >          Eina_Bool svrfreed = EINA_FALSE;
> >> >
> >> > -        cl->event_count = eina_list_remove(cl->event_count, e);
> >> > -        if (cl->host_server)
> >> > +        if ((svr) && (cl))
> >> >            {
> >> > -             Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > -             host_server->event_count =
> >> eina_list_remove(host_server->event_count, ev);
> >> > -             if ((!svr->event_count) && (svr->delete_me))
> >> > +             cl->event_count = eina_list_remove(cl->event_count, e);
> >> > +             if (cl->host_server)
> >> >                 {
> >> > -                  _ecore_con_server_free(obj);
> >> > -                  svrfreed = EINA_TRUE;
> >> > +                  Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > +                  if (host_server)
> >> > +                    host_server->event_count =
> >> eina_list_remove(host_server->event_count, ev);
> >> > +                  if ((!svr->event_count) && (svr->delete_me))
> >> > +                    {
> >> > +                       _ecore_con_server_free(obj);
> >> > +                       svrfreed = EINA_TRUE;
> >> > +                    }
> >> > +               }
> >> > +             if (!svrfreed)
> >> > +               {
> >> > +                  if (!cl->event_count)
> >> > +                    _ecore_con_client_free(e->client);
> >> >                 }
> >> > -          }
> >> > -        if (!svrfreed)
> >> > -          {
> >> > -             if (!cl->event_count)
> >> > -               _ecore_con_client_free(e->client);
> >> >            }
> >> >       }
> >> >     ecore_con_event_client_del_free(e);
> >> > @@ -2745,25 +2757,29 @@
> >> _ecore_con_event_client_write_free(Ecore_Con_Server *obj,
> >> >          Ecore_Con_Client_Data *cl = eo_data_scope_get(e->client,
> >> ECORE_CON_CLIENT_CLASS);
> >> >          Eina_Bool svrfreed = EINA_FALSE;
> >> >
> >> > -        cl->event_count = eina_list_remove(cl->event_count, e);
> >> > -        if (cl->host_server)
> >> > +        if ((svr) && (cl))
> >> >            {
> >> > -             Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > -             host_server->event_count =
> >> eina_list_remove(host_server->event_count, e);
> >> > -             if ((!svr->event_count) && (svr->delete_me))
> >> > +             cl->event_count = eina_list_remove(cl->event_count, e);
> >> > +             if (cl->host_server)
> >> >                 {
> >> > -                  _ecore_con_server_free(obj);
> >> > -                  svrfreed = EINA_TRUE;
> >> > +                  Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > +                  if (host_server)
> >> > +                    host_server->event_count =
> >> eina_list_remove(host_server->event_count, e);
> >> > +                  if ((!svr->event_count) && (svr->delete_me))
> >> > +                    {
> >> > +                       _ecore_con_server_free(obj);
> >> > +                       svrfreed = EINA_TRUE;
> >> > +                    }
> >> > +               }
> >> > +             if (!svrfreed)
> >> > +               {
> >> > +                  Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > +                  if (((!cl->event_count) && (cl->delete_me)) ||
> >> > +                      ((cl->host_server &&
> >> > +                        ((host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_UDP ||
> >> > +                            (host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_MCAST))))
> >> > +                    ecore_con_client_del(e->client);
> >> >                 }
> >> > -          }
> >> > -        if (!svrfreed)
> >> > -          {
> >> > -             Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > -             if (((!cl->event_count) && (cl->delete_me)) ||
> >> > -                 ((cl->host_server &&
> >> > -                   ((host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_UDP ||
> >> > -                    (host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_MCAST))))
> >> > -               ecore_con_client_del(e->client);
> >> >            }
> >> >       }
> >> >     ecore_con_event_client_write_free(e);
> >> > @@ -2785,25 +2801,29 @@
> >> _ecore_con_event_client_data_free(Ecore_Con_Server *obj,
> >> >          Ecore_Con_Client_Data *cl = eo_data_scope_get(e->client,
> >> ECORE_CON_CLIENT_CLASS);
> >> >          Eina_Bool svrfreed = EINA_FALSE;
> >> >
> >> > -        cl->event_count = eina_list_remove(cl->event_count, e);
> >> > -        if (cl->host_server)
> >> > -          {
> >> > -             Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > -             host_server->event_count =
> >> eina_list_remove(host_server->event_count, ev);
> >> > -          }
> >> > -        if ((!svr->event_count) && (svr->delete_me))
> >> > -          {
> >> > -             _ecore_con_server_free(obj);
> >> > -             svrfreed = EINA_TRUE;
> >> > -          }
> >> > -        if (!svrfreed)
> >> > +        if ((svr) && (cl))
> >> >            {
> >> > -             Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > -             if (((!cl->event_count) && (cl->delete_me)) ||
> >> > -                 ((cl->host_server &&
> >> > -                   ((host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_UDP ||
> >> > -                    (host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_MCAST))))
> >> > -               ecore_con_client_del(e->client);
> >> > +             cl->event_count = eina_list_remove(cl->event_count, e);
> >> > +             if (cl->host_server)
> >> > +               {
> >> > +                  Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > +                  if (host_server)
> >> > +                    host_server->event_count =
> >> eina_list_remove(host_server->event_count, ev);
> >> > +               }
> >> > +             if ((!svr->event_count) && (svr->delete_me))
> >> > +               {
> >> > +                  _ecore_con_server_free(obj);
> >> > +                  svrfreed = EINA_TRUE;
> >> > +               }
> >> > +             if (!svrfreed)
> >> > +               {
> >> > +                  Ecore_Con_Server_Data *host_server =
> >> eo_data_scope_get(cl->host_server, ECORE_CON_SERVER_CLASS);
> >> > +                  if (((!cl->event_count) && (cl->delete_me)) ||
> >> > +                      ((cl->host_server &&
> >> > +                        ((host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_UDP ||
> >> > +                            (host_server->type & ECORE_CON_TYPE) ==
> >> ECORE_CON_REMOTE_MCAST))))
> >> > +                    ecore_con_client_del(e->client);
> >> > +               }
> >> >            }
> >> >       }
> >> >     free(e->data);
> >> > @@ -2823,9 +2843,12 @@ _ecore_con_event_server_add_free(void *data
> >> EINA_UNUSED,
> >> >     if (e->server)
> >> >       {
> >> >          Ecore_Con_Server_Data *svr = eo_data_scope_get(e->server,
> >> ECORE_CON_SERVER_CLASS);
> >> > -        svr->event_count = eina_list_remove(svr->event_count, ev);
> >> > -        if ((!svr->event_count) && (svr->delete_me))
> >> > -          _ecore_con_server_free(e->server);
> >> > +        if (svr)
> >> > +          {
> >> > +             svr->event_count = eina_list_remove(svr->event_count, ev);
> >> > +             if ((!svr->event_count) && (svr->delete_me))
> >> > +               _ecore_con_server_free(e->server);
> >> > +          }
> >> >       }
> >> >     ecore_con_event_server_add_free(e);
> >> >     _ecore_con_event_count--;
> >> > @@ -2843,9 +2866,12 @@ _ecore_con_event_server_del_free(void *data
> >> EINA_UNUSED,
> >> >     if (e->server)
> >> >       {
> >> >          Ecore_Con_Server_Data *svr = eo_data_scope_get(e->server,
> >> ECORE_CON_SERVER_CLASS);
> >> > -        svr->event_count = eina_list_remove(svr->event_count, ev);
> >> > -        if (!svr->event_count)
> >> > -          _ecore_con_server_free(e->server);
> >> > +        if (svr)
> >> > +          {
> >> > +             svr->event_count = eina_list_remove(svr->event_count, ev);
> >> > +             if (!svr->event_count)
> >> > +               _ecore_con_server_free(e->server);
> >> > +          }
> >> >       }
> >> >     ecore_con_event_server_del_free(e);
> >> >     _ecore_con_event_count--;
> >> > @@ -2860,11 +2886,13 @@ _ecore_con_event_server_write_free(void *data
> >> EINA_UNUSED,
> >> >     if (e->server)
> >> >       {
> >> >          Ecore_Con_Server_Data *svr = eo_data_scope_get(e->server,
> >> ECORE_CON_SERVER_CLASS);
> >> > -        svr->event_count = eina_list_remove(svr->event_count, e);
> >> > -        if ((!svr->event_count) && (svr->delete_me))
> >> > -          _ecore_con_server_free(e->server);
> >> > +        if (svr)
> >> > +          {
> >> > +             svr->event_count = eina_list_remove(svr->event_count, e);
> >> > +             if ((!svr->event_count) && (svr->delete_me))
> >> > +               _ecore_con_server_free(e->server);
> >> > +          }
> >> >       }
> >> > -
> >> >     ecore_con_event_server_write_free(e);
> >> >     _ecore_con_event_count--;
> >> >     if ((!_ecore_con_event_count) && (!_ecore_con_init_count))
> >> > @@ -2881,9 +2909,12 @@ _ecore_con_event_server_data_free(void *data
> >> EINA_UNUSED,
> >> >     if (e->server)
> >> >       {
> >> >          Ecore_Con_Server_Data *svr = eo_data_scope_get(e->server,
> >> ECORE_CON_SERVER_CLASS);
> >> > -        svr->event_count = eina_list_remove(svr->event_count, ev);
> >> > -        if ((!svr->event_count) && (svr->delete_me))
> >> > -          _ecore_con_server_free(e->server);
> >> > +        if (svr)
> >> > +          {
> >> > +             svr->event_count = eina_list_remove(svr->event_count, ev);
> >> > +             if ((!svr->event_count) && (svr->delete_me))
> >> > +               _ecore_con_server_free(e->server);
> >> > +          }
> >> >       }
> >> >
> >> >     free(e->data);
> >> > @@ -2899,9 +2930,12 @@ _ecore_con_event_server_error_free(void *data
> >> EINA_UNUSED, Ecore_Con_Event_Serve
> >> >     if (e->server)
> >> >       {
> >> >          Ecore_Con_Server_Data *svr = eo_data_scope_get(e->server,
> >> ECORE_CON_SERVER_CLASS);
> >> > -        svr->event_count = eina_list_remove(svr->event_count, e);
> >> > -        if ((!svr->event_count) && (svr->delete_me))
> >> > -          _ecore_con_server_free(e->server);
> >> > +        if (svr)
> >> > +          {
> >> > +             svr->event_count = eina_list_remove(svr->event_count, e);
> >> > +             if ((!svr->event_count) && (svr->delete_me))
> >> > +               _ecore_con_server_free(e->server);
> >> > +          }
> >> >       }
> >> >     free(e->error);
> >> >     ecore_con_event_server_error_free(e);
> >> > @@ -2919,20 +2953,23 @@
> >> _ecore_con_event_client_error_free(Ecore_Con_Server *obj,
> >> Ecore_Con_Event_Client
> >> >          Ecore_Con_Client_Data *cl = eo_data_scope_get(e->client,
> >> ECORE_CON_CLIENT_CLASS);
> >> >          Eina_Bool svrfreed = EINA_FALSE;
> >> >
> >> > -        if (eina_list_data_find(svr->clients, e->client))
> >> > +        if ((svr) && (cl))
> >> >            {
> >> > -             cl->event_count = eina_list_remove(cl->event_count, e);
> >> > -             if ((!cl->event_count) && (cl->delete_me))
> >> > +             if (eina_list_data_find(svr->clients, e->client))
> >> >                 {
> >> > -                  _ecore_con_client_free(e->client);
> >> > -                  svrfreed = EINA_TRUE;
> >> > +                  cl->event_count = eina_list_remove(cl->event_count,
> >> > e);
> >> > +                  if ((!cl->event_count) && (cl->delete_me))
> >> > +                    {
> >> > +                       _ecore_con_client_free(e->client);
> >> > +                       svrfreed = EINA_TRUE;
> >> > +                    }
> >> > +               }
> >> > +             svr->event_count = eina_list_remove(svr->event_count, e);
> >> > +             if (!svrfreed)
> >> > +               {
> >> > +                  if ((!svr->event_count) && (svr->delete_me))
> >> > +                    _ecore_con_server_free(obj);
> >> >                 }
> >> > -          }
> >> > -        svr->event_count = eina_list_remove(svr->event_count, e);
> >> > -        if (!svrfreed)
> >> > -          {
> >> > -             if ((!svr->event_count) && (svr->delete_me))
> >> > -               _ecore_con_server_free(obj);
> >> >            }
> >> >       }
> >> >     free(e->error);
> >> > @@ -2950,6 +2987,7 @@ _ecore_con_lookup_done(void *data,
> >> >     Ecore_Con_Lookup *lk;
> >> >
> >> >     Ecore_Con_Server_Data *svr = eo_data_scope_get(obj,
> >> ECORE_CON_SERVER_CLASS);
> >> > +   if (!svr) return;
> >> >     lk = svr->data;
> >> >
> >> >     if (infos)
> >> >
> >> > --
> >> >
> >> >
> >> >
> >> ------------------------------------------------------------------------------
> >> Monitor 25 network devices or servers for free with OpManager!
> >> OpManager is web-based network management software that monitors
> >> network devices and physical & virtual servers, alerts via email & sms
> >> for fault. Monitor 25 devices for free with no restriction. Download now
> >> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
> >> _______________________________________________
> >> 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]
> >
> >
> 
> 
> 
> -- 
> Cedric BAIL
> 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    [email protected]


------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors 
network devices and physical & virtual servers, alerts via email & sms 
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to