[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [56063] trunk/blender/source/blender: 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).

Lukas Tönne lukas.toenne at gmail.com
Fri Apr 19 12:16:01 CEST 2013


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 <montagne29 at wanadoo.fr>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
> Bf-blender-cvs at blender.org
> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>


More information about the Bf-committers mailing list