On 25/06/15 10:03, Cedric BAIL wrote:
> 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
> 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.
It's misleading (maybe) but faster. We could change that, but it'll be
significantly slower, and it's a common enough operation and mostly done
in a way that doesn't need the check. Sorry, it's low-level c, we make
optimisations.
Checking if the object is deleted this way is a horrible hack anyway. It
will print errors to console, and it only works and doesn't crash
because of our fail-safe mechanisms. It's not normal code path.
You could maybe use eo_isa() which is a bit more resilient, but still
has the same hackiness to 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]
>>
>>
>
>
>
------------------------------------------------------------------------------
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
