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
