Hi, It breaks every search at the moment. I know rna code is complex, but I don't understand a line of this... I also don't understand the description of the feature. What's the use case here?
One thing for sure: you cannot store stuff in buttons, storage should be done outside, by callers. The crash is because stored rna properties get freed elsewhere. -Ton- ------------------------------------------------------------------------ Ton Roosendaal Blender Foundation [email protected] www.blender.org Blender Institute Entrepotdok 57A 1018AD Amsterdam The Netherlands On 19 Apr, 2013, at 12:16, Lukas Tönne wrote: > This commit seems to have broken the > WM_enum_search_invoke function, used as wm.invoke_search_popup(self) in the > node.add_search operator. Invoking the search popup (via ctrl+A in node > editor -> "Search ...") and then exiting either by moving away the mouse or > by executing one of the items leads to free'd memory access and crash: > > http://www.pasteall.org/41525 > > Reported by Francesco Siddi via Sergey in IRC. > > > On Mon, Apr 15, 2013 at 5:01 PM, Bastien Montagne > <[email protected]>wrote: > >> Revision: 56063 >> >> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=56063 >> Author: mont29 >> Date: 2013-04-15 15:01:12 +0000 (Mon, 15 Apr 2013) >> Log Message: >> ----------- >> Fix: when using a search menu with an operator's enum prop, the operator >> was previously always executed with default options (appart from the >> search-set enum, of course). Now we store the op's properties in search >> button, so that you can specify non-default options (as it was already >> possible with e.g. pop-up menu from an operator's enum prop). >> >> To achieve this, some code (callbacks and search button creation) was >> moved from wm_operators.c to interface/interface.c, and a new UI function >> was added, uiDefSearchButO_ptr. >> >> Note: This new code uses the fact that uiButHandleFunc callbacks get >> executed before operator when one of its arg is the button itself! >> >> Many thanks to Campbell who helped me a lot with this patch! >> >> Cleanup: also removed two unused pointers from uiBut struct. >> >> Modified Paths: >> -------------- >> trunk/blender/source/blender/editors/include/UI_interface.h >> trunk/blender/source/blender/editors/interface/interface.c >> trunk/blender/source/blender/editors/interface/interface_handlers.c >> trunk/blender/source/blender/editors/interface/interface_intern.h >> trunk/blender/source/blender/windowmanager/intern/wm_operators.c >> >> Modified: trunk/blender/source/blender/editors/include/UI_interface.h >> =================================================================== >> --- trunk/blender/source/blender/editors/include/UI_interface.h 2013-04-15 >> 14:55:42 UTC (rev 56062) >> +++ trunk/blender/source/blender/editors/include/UI_interface.h 2013-04-15 >> 15:01:12 UTC (rev 56063) >> @@ -605,6 +605,9 @@ >> uiBut *uiDefHotKeyevtButS(uiBlock *block, int retval, const char *str, >> int x, int y, short width, short height, short *keypoin, short *modkeypoin, >> const char *tip); >> >> uiBut *uiDefSearchBut(uiBlock *block, void *arg, int retval, int icon, >> int maxlen, int x, int y, short width, short height, float a1, float a2, >> const char *tip); >> +uiBut *uiDefSearchButO_ptr(uiBlock *block, struct wmOperatorType *ot, >> IDProperty *properties, >> + void *arg, int retval, int icon, int maxlen, >> int x, int y, >> + short width, short height, float a1, float a2, >> const char *tip); >> >> uiBut *uiDefAutoButR(uiBlock *block, struct PointerRNA *ptr, struct >> PropertyRNA *prop, int index, const char *name, int icon, int x1, int y1, >> int x2, int y2); >> int uiDefAutoButsRNA(uiLayout *layout, struct PointerRNA *ptr, bool >> (*check_prop)(struct PointerRNA *, struct PropertyRNA *), const char >> label_align); >> >> Modified: trunk/blender/source/blender/editors/interface/interface.c >> =================================================================== >> --- trunk/blender/source/blender/editors/interface/interface.c 2013-04-15 >> 14:55:42 UTC (rev 56062) >> +++ trunk/blender/source/blender/editors/interface/interface.c 2013-04-15 >> 15:01:12 UTC (rev 56063) >> @@ -3832,6 +3832,82 @@ >> } >> } >> >> +/* Callbacks for operator search button. */ >> +static void operator_enum_search_cb(const struct bContext *C, void *but, >> const char *str, uiSearchItems *items) >> +{ >> + wmOperatorType *ot = ((uiBut *)but)->optype; >> + PropertyRNA *prop = ot->prop; >> + >> + if (prop == NULL) { >> + printf("%s: %s has no enum property set\n", >> + __func__, ot->idname); >> + } >> + else if (RNA_property_type(prop) != PROP_ENUM) { >> + printf("%s: %s \"%s\" is not an enum property\n", >> + __func__, ot->idname, >> RNA_property_identifier(prop)); >> + } >> + else { >> + PointerRNA *ptr = uiButGetOperatorPtrRNA(but); /* Will >> create it if needed! */ >> + EnumPropertyItem *item, *item_array; >> + int do_free; >> + >> + RNA_property_enum_items((bContext *)C, ptr, prop, >> &item_array, NULL, &do_free); >> + >> + for (item = item_array; item->identifier; item++) { >> + /* note: need to give the index rather than the >> identifier because the enum can be freed */ >> + if (BLI_strcasestr(item->name, str)) { >> + if (false == uiSearchItemAdd(items, >> item->name, SET_INT_IN_POINTER(item->value), 0)) >> + break; >> + } >> + } >> + >> + if (do_free) >> + MEM_freeN(item_array); >> + } >> +} >> + >> +static void operator_enum_call_cb(struct bContext *UNUSED(C), void *but, >> void *arg2) >> +{ >> + wmOperatorType *ot = ((uiBut *)but)->optype; >> + PointerRNA *opptr = uiButGetOperatorPtrRNA(but); /* Will create >> it if needed! */ >> + >> + if (ot) { >> + if (ot->prop) { >> + RNA_property_enum_set(opptr, ot->prop, >> GET_INT_FROM_POINTER(arg2)); >> + /* We do not call op from here, will be called by >> button code. >> + * ui_apply_but_funcs_after() (in >> interface_handlers.c) called this func before checking operators, >> + * because one of its parameters is the button >> itself! >> + */ >> + } >> + else { >> + printf("%s: op->prop for '%s' is NULL\n", >> __func__, ot->idname); >> + } >> + } >> +} >> + >> +/* Same parameters as for uiDefSearchBut, with an additional operator >> pointer, from where to get property to search, >> + * operator type, and operator porperties. */ >> +uiBut *uiDefSearchButO_ptr(uiBlock *block, wmOperatorType *ot, IDProperty >> *properties, >> + void *arg, int retval, int icon, int maxlen, >> int x, int y, >> + short width, short height, float a1, float a2, >> const char *tip) >> +{ >> + uiBut *but; >> + >> + but = uiDefSearchBut(block, arg, retval, icon, maxlen, x, y, >> width, height, a1, a2, tip); >> + uiButSetSearchFunc(but, operator_enum_search_cb, but, >> operator_enum_call_cb, NULL); >> + >> + but->optype = ot; >> + but->opcontext = WM_OP_EXEC_DEFAULT; >> + >> + if (properties) { >> + PointerRNA *ptr = uiButGetOperatorPtrRNA(but); >> + /* Copy pointer. */ >> + RNA_pointer_create(NULL, ot->srna, properties, ptr); >> + } >> + >> + return but; >> +} >> + >> /* push a new event onto event queue to activate the given button >> * (usually a text-field) upon entering a popup >> */ >> >> Modified: >> trunk/blender/source/blender/editors/interface/interface_handlers.c >> =================================================================== >> --- trunk/blender/source/blender/editors/interface/interface_handlers.c >> 2013-04-15 14:55:42 UTC (rev 56062) >> +++ trunk/blender/source/blender/editors/interface/interface_handlers.c >> 2013-04-15 15:01:12 UTC (rev 56063) >> @@ -199,7 +199,6 @@ >> uiButHandleFunc func; >> void *func_arg1; >> void *func_arg2; >> - void *func_arg3; >> >> uiButHandleNFunc funcN; >> void *func_argN; >> @@ -386,7 +385,6 @@ >> >> after->func_arg1 = but->func_arg1; >> after->func_arg2 = but->func_arg2; >> - after->func_arg3 = but->func_arg3; >> >> after->funcN = but->funcN; >> after->func_argN = MEM_dupallocN(but->func_argN); >> >> Modified: trunk/blender/source/blender/editors/interface/interface_intern.h >> =================================================================== >> --- trunk/blender/source/blender/editors/interface/interface_intern.h >> 2013-04-15 14:55:42 UTC (rev 56062) >> +++ trunk/blender/source/blender/editors/interface/interface_intern.h >> 2013-04-15 15:01:12 UTC (rev 56063) >> @@ -201,7 +201,6 @@ >> uiButHandleFunc func; >> void *func_arg1; >> void *func_arg2; >> - void *func_arg3; >> >> uiButHandleNFunc funcN; >> void *func_argN; >> @@ -251,7 +250,6 @@ >> >> /* Operator data */ >> struct wmOperatorType *optype; >> - struct IDProperty *opproperties; >> struct PointerRNA *opptr; >> short opcontext; >> unsigned char menu_key; /* 'a'-'z', always lower case */ >> >> Modified: trunk/blender/source/blender/windowmanager/intern/wm_operators.c >> =================================================================== >> --- trunk/blender/source/blender/windowmanager/intern/wm_operators.c >> 2013-04-15 14:55:42 UTC (rev 56062) >> +++ trunk/blender/source/blender/windowmanager/intern/wm_operators.c >> 2013-04-15 15:01:12 UTC (rev 56063) >> @@ -939,58 +939,6 @@ >> >> >> /* generic enum search invoke popup */ >> -static void operator_enum_search_cb(const struct bContext *C, void >> *arg_ot, const char *str, uiSearchItems *items) >> -{ >> - wmOperatorType *ot = (wmOperatorType *)arg_ot; >> - PropertyRNA *prop = ot->prop; >> - >> - if (prop == NULL) { >> - printf("%s: %s has no enum property set\n", >> - __func__, ot->idname); >> - } >> - else if (RNA_property_type(prop) != PROP_ENUM) { >> - printf("%s: %s \"%s\" is not an enum property\n", >> - __func__, ot->idname, >> RNA_property_identifier(prop)); >> - } >> - else { >> - PointerRNA ptr; >> - >> - EnumPropertyItem *item, *item_array; >> - int do_free; >> - >> - RNA_pointer_create(NULL, ot->srna, NULL, &ptr); >> - RNA_property_enum_items((bContext *)C, &ptr, prop, >> &item_array, NULL, &do_free); >> - >> - for (item = item_array; item->identifier; item++) { >> - /* note: need to give the index rather than the >> identifier because the enum can be freed */ >> - if (BLI_strcasestr(item->name, str)) >> - if (false == uiSearchItemAdd(items, >> item->name, SET_INT_IN_POINTER(item->value), 0)) >> - break; >> - } >> - >> - if (do_free) >> - MEM_freeN(item_array); >> - } >> -} >> - >> -static void operator_enum_call_cb(struct bContext *C, void *arg1, void >> *arg2) >> -{ >> - wmOperatorType *ot = arg1; >> - >> - if (ot) { >> - if (ot->prop) { >> - PointerRNA props_ptr; >> - WM_operator_properties_create_ptr(&props_ptr, ot); >> - RNA_property_enum_set(&props_ptr, ot->prop, >> GET_INT_FROM_POINTER(arg2)); >> - WM_operator_name_call(C, ot->idname, >> WM_OP_EXEC_DEFAULT, &props_ptr); >> - WM_operator_properties_free(&props_ptr); >> - } >> - else { >> - printf("%s: op->prop for '%s' is NULL\n", >> __func__, ot->idname); >> - } >> - } >> -} >> - >> static uiBlock *wm_enum_search_menu(bContext *C, ARegion *ar, void >> *arg_op) >> { >> static char search[256] = ""; >> @@ -1006,8 +954,8 @@ >> #if 0 /* ok, this isn't so easy... */ >> uiDefBut(block, LABEL, 0, RNA_struct_ui_name(op->type->srna), 10, >> 10, 180, UI_UNIT_Y, NULL, 0.0, 0.0, 0, 0, ""); >> #endif >> - but = uiDefSearchBut(block, search, 0, ICON_VIEWZOOM, >> sizeof(search), 10, 10, 9 * UI_UNIT_X, UI_UNIT_Y, 0, 0, ""); >> - uiButSetSearchFunc(but, operator_enum_search_cb, op->type, >> operator_enum_call_cb, NULL); >> + but = uiDefSearchButO_ptr(block, op->type, op->ptr->data, search, >> 0, ICON_VIEWZOOM, sizeof(search), >> + 10, 10, 9 * UI_UNIT_X, UI_UNIT_Y, 0, 0, >> ""); >> >> /* fake button, it holds space for search items */ >> uiDefBut(block, LABEL, 0, "", 10, 10 - uiSearchBoxHeight(), >> uiSearchBoxWidth(), uiSearchBoxHeight(), NULL, 0, 0, 0, 0, NULL); >> >> _______________________________________________ >> Bf-blender-cvs mailing list >> [email protected] >> http://lists.blender.org/mailman/listinfo/bf-blender-cvs >> > _______________________________________________ > Bf-committers mailing list > [email protected] > http://lists.blender.org/mailman/listinfo/bf-committers _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
