hm, actually this patch :P --lf
On Wed, Dec 9, 2009 at 5:49 PM, Luis Felipe Strano Moraes <luis.str...@gmail.com> wrote: > Gustavo, > > the macro you created ELM_LIST_ITEM_CHECK_DELETED_RETURN didn't actually check > for NULL (though the message did imply that). The attached patch fixes > that (and fixes about > 10 or more null pointer dereferences from the latest clang tests). > > --lf > > > On Wed, Dec 2, 2009 at 4:59 PM, Enlightenment SVN > <no-re...@enlightenment.org> wrote: >> Log: >> fix couple of list issues while deleting during item usage. >> >> now list marks itself as "walking" every time it's usign wd->items, >> this way any call to clear or deletion (elm_list_item_del()) will be >> postponed using the wd->to_delete list. >> >> please let me know of any bugs you may find. >> >> >> Author: barbieri >> Date: 2009-12-02 10:59:41 -0800 (Wed, 02 Dec 2009) >> New Revision: 44124 >> >> Modified: >> trunk/TMP/st/elementary/src/lib/elm_list.c >> >> Modified: trunk/TMP/st/elementary/src/lib/elm_list.c >> =================================================================== >> --- trunk/TMP/st/elementary/src/lib/elm_list.c 2009-12-02 18:47:34 UTC (rev >> 44123) >> +++ trunk/TMP/st/elementary/src/lib/elm_list.c 2009-12-02 18:59:41 UTC (rev >> 44124) >> @@ -5,10 +5,12 @@ >> >> struct _Widget_Data >> { >> - Evas_Object *scr, *box; >> - Eina_List *items, *selected; >> + Evas_Object *scr, *box, *self; >> + Eina_List *items, *selected, *to_delete; >> Elm_List_Mode mode; >> Evas_Coord minw[2], minh[2]; >> + int walking; >> + Eina_Bool fix_pending : 1; >> Eina_Bool on_hold : 1; >> Eina_Bool multi : 1; >> Eina_Bool always_select : 1; >> @@ -26,6 +28,7 @@ >> void (*del_cb) (void *data, Evas_Object *obj, void *event_info); >> const void *data; >> Ecore_Timer *long_timer; >> + Eina_Bool deleted : 1; >> Eina_Bool even : 1; >> Eina_Bool is_even : 1; >> Eina_Bool fixed : 1; >> @@ -41,27 +44,132 @@ >> static void _changed_size_hints(void *data, Evas *e, Evas_Object *obj, void >> *event_info); >> static void _sub_del(void *data, Evas_Object *obj, void *event_info); >> static void _fix_items(Evas_Object *obj); >> +static void _mouse_down(void *data, Evas *evas, Evas_Object *obj, void >> *event_info); >> +static void _mouse_up(void *data, Evas *evas, Evas_Object *obj, void >> *event_info); >> +static void _mouse_move(void *data, Evas *evas, Evas_Object *obj, void >> *event_info); >> >> +#define ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, ...) \ >> + if ((it) && ((it)->deleted)) \ >> + { \ >> + fprintf(stderr, "ERROR: %s:%d:%s() "#it" is NULL.\n", \ >> + __FILE__, __LINE__, __FUNCTION__); \ >> + return __VA_ARGS__; \ >> + } >> + >> + >> + >> +static inline void >> +_elm_list_item_call_del_cb(Elm_List_Item *it) >> +{ >> + if (it->del_cb) it->del_cb((void *)it->data, it->obj, it); >> +} >> + >> +static inline void >> +_elm_list_item_free(Elm_List_Item *it) >> +{ >> + evas_object_event_callback_del_full >> + (it->base, EVAS_CALLBACK_MOUSE_DOWN, _mouse_down, it); >> + evas_object_event_callback_del_full >> + (it->base, EVAS_CALLBACK_MOUSE_UP, _mouse_up, it); >> + evas_object_event_callback_del_full >> + (it->base, EVAS_CALLBACK_MOUSE_MOVE, _mouse_move, it); >> + >> + if (it->icon) >> + evas_object_event_callback_del_full >> + (it->icon, EVAS_CALLBACK_CHANGED_SIZE_HINTS, >> + _changed_size_hints, it->obj); >> + >> + if (it->end) >> + evas_object_event_callback_del_full >> + (it->end, EVAS_CALLBACK_CHANGED_SIZE_HINTS, >> + _changed_size_hints, it->obj); >> + >> + eina_stringshare_del(it->label); >> + >> + if (it->long_timer) ecore_timer_del(it->long_timer); >> + if (it->icon) evas_object_del(it->icon); >> + if (it->end) evas_object_del(it->end); >> + if (it->base) evas_object_del(it->base); >> + free(it); >> +} >> + >> static void >> +_elm_list_process_deletions(Widget_Data *wd) >> +{ >> + Elm_List_Item *it; >> + >> + wd->walking++; // avoid nested deletion and also _sub_del() fix_items >> + >> + EINA_LIST_FREE(wd->to_delete, it) >> + { >> + _elm_list_item_call_del_cb(it); >> + >> + wd->items = eina_list_remove_list(wd->items, it->node); >> + _elm_list_item_free(it); >> + } >> + >> + wd->walking--; >> +} >> + >> +static inline void >> +_elm_list_walk(Widget_Data *wd) >> +{ >> + if (wd->walking < 0) >> + { >> + fprintf(stderr, "ERROR: walking was negative. fixed!\n"); >> + wd->walking = 0; >> + } >> + wd->walking++; >> +} >> + >> +static inline void >> +_elm_list_unwalk(Widget_Data *wd) >> +{ >> + wd->walking--; >> + if (wd->walking < 0) >> + { >> + fprintf(stderr, "ERROR: walking became negative. fixed!\n"); >> + wd->walking = 0; >> + } >> + >> + if (wd->walking) >> + return; >> + >> + if (wd->to_delete) >> + _elm_list_process_deletions(wd); >> + >> + if (wd->fix_pending) >> + { >> + wd->fix_pending = EINA_FALSE; >> + _fix_items(wd->self); >> + _sizing_eval(wd->self); >> + } >> +} >> + >> +static void >> _del_hook(Evas_Object *obj) >> { >> Widget_Data *wd = elm_widget_data_get(obj); >> Elm_List_Item *it; >> + Eina_List *n; >> >> + if (wd->walking != 0) >> + fprintf(stderr, "ERROR: list deleted while walking.\n"); >> + >> + _elm_list_walk(wd); >> + >> + EINA_LIST_FOREACH(wd->items, n, it) >> + _elm_list_item_call_del_cb(it); >> + >> + _elm_list_unwalk(wd); >> + if (wd->to_delete) >> + fprintf(stderr, "ERROR: leaking nodes!\n"); >> + >> EINA_LIST_FREE(wd->items, it) >> - { >> - if (it->del_cb) it->del_cb((void *)it->data, it->obj, it); >> - if (it->long_timer) ecore_timer_del(it->long_timer); >> - eina_stringshare_del(it->label); >> - if (!it->fixed) >> - { >> - if (it->icon) evas_object_del(it->icon); >> - if (it->end) evas_object_del(it->end); >> - } >> - if (it->base) evas_object_del(it->base); >> - free(it); >> - } >> + _elm_list_item_free(it); >> + >> eina_list_free(wd->selected); >> + >> free(wd); >> } >> >> @@ -110,8 +218,13 @@ >> evas_object_event_callback_del_full(sub, >> EVAS_CALLBACK_CHANGED_SIZE_HINTS, >> _changed_size_hints, obj); >> - _fix_items(obj); >> - _sizing_eval(obj); >> + if (!wd->walking) >> + { >> + _fix_items(obj); >> + _sizing_eval(obj); >> + } >> + else >> + wd->fix_pending = EINA_TRUE; >> break; >> } >> } >> @@ -123,12 +236,17 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> const char *selectraise; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> if (it->hilighted) return; >> + _elm_list_walk(wd); >> + >> edje_object_signal_emit(it->base, "elm,state,selected", "elm"); >> selectraise = edje_object_data_get(it->base, "selectraise"); >> if ((selectraise) && (!strcmp(selectraise, "on"))) >> evas_object_raise(it->base); >> it->hilighted = EINA_TRUE; >> + >> + _elm_list_unwalk(wd); >> } >> >> static void >> @@ -137,6 +255,7 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> const char *selectraise; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> if (it->selected) >> { >> if (wd->always_select) goto call; >> @@ -145,8 +264,12 @@ >> it->selected = EINA_TRUE; >> wd->selected = eina_list_append(wd->selected, it); >> call: >> + _elm_list_walk(wd); >> + >> if (it->func) it->func((void *)it->data, it->obj, it); >> evas_object_smart_callback_call(it->obj, "selected", it); >> + >> + _elm_list_unwalk(wd); >> } >> >> static void >> @@ -155,7 +278,10 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> const char *stacking, *selectraise; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> if (!it->hilighted) return; >> + _elm_list_walk(wd); >> + >> edje_object_signal_emit(it->base, "elm,state,unselected", "elm"); >> stacking = edje_object_data_get(it->base, "stacking"); >> selectraise = edje_object_data_get(it->base, "selectraise"); >> @@ -171,6 +297,8 @@ >> wd->selected = eina_list_remove(wd->selected, it); >> evas_object_smart_callback_call(it->obj, "unselected", it); >> } >> + >> + _elm_list_unwalk(wd); >> } >> >> static void >> @@ -180,6 +308,8 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> Evas_Event_Mouse_Move *ev = event_info; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (ev->event_flags & EVAS_EVENT_FLAG_ON_HOLD) >> { >> if (!wd->on_hold) >> @@ -202,6 +332,9 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> >> it->long_timer = NULL; >> + >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, 0); >> + >> wd->longpressed = EINA_TRUE; >> evas_object_smart_callback_call(it->obj, "longpressed", it); >> return 0; >> @@ -214,6 +347,8 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> Evas_Event_Mouse_Down *ev = event_info; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (ev->button != 1) return; >> if (ev->event_flags & EVAS_EVENT_FLAG_ON_HOLD) wd->on_hold = EINA_TRUE; >> else wd->on_hold = EINA_FALSE; >> @@ -234,6 +369,8 @@ >> Widget_Data *wd = elm_widget_data_get(it->obj); >> Evas_Event_Mouse_Up *ev = event_info; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (ev->button != 1) return; >> if (ev->event_flags & EVAS_EVENT_FLAG_ON_HOLD) wd->on_hold = EINA_TRUE; >> else wd->on_hold = EINA_FALSE; >> @@ -254,6 +391,9 @@ >> wd->wasselected = 0; >> return; >> } >> + >> + _elm_list_walk(wd); // watch out "return" before unwalk! >> + >> if (wd->multi) >> { >> if (!it->selected) >> @@ -283,6 +423,8 @@ >> _item_select(it); >> } >> } >> + >> + _elm_list_unwalk(wd); >> } >> >> static Elm_List_Item * >> @@ -333,9 +475,18 @@ >> int i, redo = 0; >> const char *style = elm_widget_style_get(obj); >> >> + if (wd->walking) >> + { >> + wd->fix_pending = EINA_TRUE; >> + return; >> + } >> + >> + _elm_list_walk(wd); // watch out "return" before unwalk! >> + >> EINA_LIST_FOREACH(wd->items, l, it) >> { >> Evas_Coord mw, mh; >> + if (it->deleted) continue; >> if (it->icon) >> { >> evas_object_size_hint_min_get(it->icon, &mw, &mh); >> @@ -361,6 +512,7 @@ >> i = 0; >> EINA_LIST_FOREACH(wd->items, l, it) >> { >> + if (it->deleted) continue; >> it->even = i & 0x1; >> if ((it->even != it->is_even) || (!it->fixed) || (redo)) >> { >> @@ -415,7 +567,12 @@ >> } >> if (!it->fixed) >> { >> + // this may call up user and it may modify the list item >> + // but we're safe as we're flagged as walking. >> + // just don't process further >> edje_object_message_signal_process(it->base); >> + if (it->deleted) >> + continue; >> mw = mh = -1; >> elm_coords_finger_size_adjust(1, &mw, 1, &mh); >> edje_object_size_min_restricted_calc(it->base, &mw, &mh, >> mw, mh); >> @@ -427,7 +584,13 @@ >> { >> const char *selectraise; >> >> + // this may call up user and it may modify the list item >> + // but we're safe as we're flagged as walking. >> + // just don't process further >> edje_object_signal_emit(it->base, "elm,state,selected", >> "elm"); >> + if (it->deleted) >> + continue; >> + >> selectraise = edje_object_data_get(it->base, "selectraise"); >> if ((selectraise) && (!strcmp(selectraise, "on"))) >> evas_object_raise(it->base); >> @@ -438,6 +601,9 @@ >> } >> i++; >> } >> + >> + _elm_list_unwalk(wd); >> + >> mw = 0; mh = 0; >> evas_object_size_hint_min_get(wd->box, &mw, &mh); >> if (wd->mode == ELM_LIST_LIMIT) >> @@ -492,7 +658,7 @@ >> >> wd = ELM_NEW(Widget_Data); >> e = evas_object_evas_get(parent); >> - obj = elm_widget_add(e); >> + wd->self = obj = elm_widget_add(e); >> elm_widget_type_set(obj, "list"); >> elm_widget_sub_object_add(parent, obj); >> elm_widget_on_focus_hook_set(obj, _on_focus_hook, NULL); >> @@ -555,6 +721,7 @@ >> Elm_List_Item *it; >> >> if ((!before) || (!before->node)) return NULL; >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(before, NULL); >> >> wd = elm_widget_data_get(obj); >> it = _item_new(obj, label, icon, end, func, data); >> @@ -571,6 +738,7 @@ >> Elm_List_Item *it; >> >> if ((!after) || (!after->node)) return NULL; >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(after, NULL); >> >> wd = elm_widget_data_get(obj); >> it = _item_new(obj, label, icon, end, func, data); >> @@ -584,9 +752,40 @@ >> elm_list_clear(Evas_Object *obj) >> { >> Widget_Data *wd = elm_widget_data_get(obj); >> + Elm_List_Item *it; >> >> - while (wd->items) >> - elm_list_item_del(wd->items->data); >> + if (!wd->items) >> + return; >> + >> + eina_list_free(wd->selected); >> + wd->selected = NULL; >> + >> + if (wd->walking > 0) >> + { >> + Eina_List *n; >> + Elm_List_Item *it; >> + EINA_LIST_FOREACH(wd->items, n, it) >> + { >> + if (it->deleted) >> + continue; >> + it->deleted = EINA_TRUE; >> + wd->to_delete = eina_list_append(wd->to_delete, it); >> + } >> + return; >> + } >> + >> + _elm_list_walk(wd); >> + >> + EINA_LIST_FREE(wd->items, it) >> + { >> + _elm_list_item_call_del_cb(it); >> + _elm_list_item_free(it); >> + } >> + >> + _elm_list_unwalk(wd); >> + >> + _fix_items(obj); >> + _sizing_eval(obj); >> } >> >> EAPI void >> @@ -656,9 +855,13 @@ >> { >> Widget_Data *wd = elm_widget_data_get(it->obj); >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> selected = !!selected; >> if (it->selected == selected) return; >> >> + _elm_list_walk(wd); >> + >> if (selected) >> { >> if (!wd->multi) >> @@ -671,6 +874,8 @@ >> } >> else >> _item_unselect(it); >> + >> + _elm_list_unwalk(wd); >> } >> >> EAPI void >> @@ -680,6 +885,8 @@ >> Evas_Coord bx, by, bw, bh; >> Evas_Coord x, y, w, h; >> >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> evas_object_geometry_get(wd->box, &bx, &by, &bw, &bh); >> evas_object_geometry_get(it->base, &x, &y, &w, &h); >> x -= bx; >> @@ -692,15 +899,26 @@ >> { >> Widget_Data *wd = elm_widget_data_get(it->obj); >> >> - if (it->del_cb) it->del_cb((void *)it->data, it->obj, it); >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (it->selected) _item_unselect(it); >> + >> + if (wd->walking > 0) >> + { >> + if (it->deleted) return; >> + it->deleted = EINA_TRUE; >> + wd->to_delete = eina_list_append(wd->to_delete, it); >> + return; >> + } >> + >> wd->items = eina_list_remove_list(wd->items, it->node); >> - eina_stringshare_del(it->label); >> - if (it->long_timer) ecore_timer_del(it->long_timer); >> - if (it->icon) evas_object_del(it->icon); >> - if (it->end) evas_object_del(it->end); >> - if (it->base) evas_object_del(it->base); >> - free(it); >> + >> + _elm_list_walk(wd); >> + >> + _elm_list_item_call_del_cb(it); >> + _elm_list_item_free(it); >> + >> + _elm_list_unwalk(wd); >> } >> >> /** >> @@ -714,18 +932,24 @@ >> EAPI void >> elm_list_item_del_cb_set(Elm_List_Item *it, void (*func)(void *data, >> Evas_Object *obj, void *event_info)) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> it->del_cb = func; >> } >> >> EAPI void * >> elm_list_item_data_get(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> return (void *)it->data; >> } >> >> EAPI Evas_Object * >> elm_list_item_icon_get(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> if (it->dummy_icon) return NULL; >> return it->icon; >> } >> @@ -733,6 +957,8 @@ >> EAPI void >> elm_list_item_icon_set(Elm_List_Item *it, Evas_Object *icon) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (it->icon == icon) return; >> if (it->dummy_icon && !icon) return; >> if (it->dummy_icon) evas_object_del(it->icon); >> @@ -750,6 +976,8 @@ >> EAPI Evas_Object * >> elm_list_item_end_get(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> if (it->dummy_end) return NULL; >> return it->end; >> } >> @@ -757,6 +985,8 @@ >> EAPI void >> elm_list_item_end_set(Elm_List_Item *it, Evas_Object *end) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (it->end == end) return; >> if (it->dummy_end && !end) return; >> if (it->dummy_end) evas_object_del(it->end); >> @@ -774,18 +1004,24 @@ >> EAPI Evas_Object * >> elm_list_item_base_get(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> return it->base; >> } >> >> EAPI const char * >> elm_list_item_label_get(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> return it->label; >> } >> >> EAPI void >> elm_list_item_label_set(Elm_List_Item *it, const char *text) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it); >> + >> if (!eina_stringshare_replace(&it->label, text)) return; >> if (it->base) >> edje_object_part_text_set(it->base, "elm.text", it->label); >> @@ -794,6 +1030,8 @@ >> EAPI Elm_List_Item * >> elm_list_item_prev(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> if (it->node->prev) >> return it->node->prev->data; >> else >> @@ -803,6 +1041,8 @@ >> EAPI Elm_List_Item * >> elm_list_item_next(const Elm_List_Item *it) >> { >> + ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, NULL); >> + >> if (it->node->next) >> return it->node->next->data; >> else >> >> >> ------------------------------------------------------------------------------ >> Join us December 9, 2009 for the Red Hat Virtual Experience, >> a free event focused on virtualization and cloud computing. >> Attend in-depth sessions from your desk. Your couch. Anywhere. >> http://p.sf.net/sfu/redhat-sfdev2dev >> _______________________________________________ >> enlightenment-svn mailing list >> enlightenment-...@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/enlightenment-svn >> > > > > -- > "Sometimes you gotta look reality in the face and say no!" -- Benjamin > Gonzalez > -- "Sometimes you gotta look reality in the face and say no!" -- Benjamin Gonzalez
Index: trunk/TMP/st/elementary/src/lib/elm_list.c =================================================================== --- trunk/TMP/st/elementary/src/lib/elm_list.c (revision 44324) +++ trunk/TMP/st/elementary/src/lib/elm_list.c (working copy) @@ -48,12 +48,18 @@ static void _mouse_up(void *data, Evas *evas, Evas_Object *obj, void *event_info); static void _mouse_move(void *data, Evas *evas, Evas_Object *obj, void *event_info); -#define ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, ...) \ - if ((it) && ((it)->deleted)) \ - { \ - fprintf(stderr, "ERROR: %s:%d:%s() "#it" is NULL.\n", \ - __FILE__, __LINE__, __FUNCTION__); \ - return __VA_ARGS__; \ +#define ELM_LIST_ITEM_CHECK_DELETED_RETURN(it, ...) \ + if (!it) \ + { \ + fprintf(stderr, "ERROR: %s:%d:%s() "#it" is NULL.\n", \ + __FILE__, __LINE__, __FUNCTION__); \ + return __VA_ARGS__; \ + } \ + else if (it->deleted) \ + { \ + fprintf(stderr, "ERROR: %s:%d:%s() "#it" has been DELETED.\n", \ + __FILE__, __LINE__, __FUNCTION__); \ + return __VA_ARGS__; \ }
------------------------------------------------------------------------------ Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev
_______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel