Bugfix [#34836] Crash when driver variable has path == 'data'
authorJoshua Leung <aligorith@gmail.com>
Mon, 22 Apr 2013 13:22:07 +0000 (13:22 +0000)
committerJoshua Leung <aligorith@gmail.com>
Mon, 22 Apr 2013 13:22:07 +0000 (13:22 +0000)
Most of the places which relied on RNA_path_resolve() did so believing that if
it returned true, that it had found a valid property, and that the returned
pointer+property combination would be what the path referred to. However, it
turns out that if the property at the end of the path turns out to be a
"pointer" property (e.g. "data" for Object.data), this would automatically
become the pointer part, while the prop part would be set to null. Hence, if a
user accidentally (or otherwise) specifies a path for the single-property driver
variable type like this, then Blender would crash.

This commit introduces two convenience functions - RNA_path_resolve_property()
and RNA_path_resolve_property_full() - which mirror/wrap the existing
RNA_path_resolve() functions. The only difference though is that these include a
check to ensure that what was found from resolving the path was in fact a
property (they only return true iff this is the case), and make it explicitly
clear in the name that this is what they will do so that there's no further
confusion. It is possible to do without these wrapper functions by doing these
checks inline, but the few cases that had been patched already were pretty
hideous looking specimens. Using these just make it clearer and simpler for all.

I've also beefed up the docs on these a bit, and changed these to using bools.

15 files changed:
source/blender/blenkernel/intern/anim_sys.c
source/blender/blenkernel/intern/fcurve.c
source/blender/editors/animation/anim_channels_defines.c
source/blender/editors/animation/anim_deps.c
source/blender/editors/animation/anim_draw.c
source/blender/editors/animation/anim_ipo_utils.c
source/blender/editors/animation/drivers.c
source/blender/editors/animation/keyframing.c
source/blender/editors/animation/keyingsets.c
source/blender/editors/armature/pose_slide.c
source/blender/editors/interface/interface_ops.c
source/blender/editors/space_graph/graph_buttons.c
source/blender/makesrna/RNA_access.h
source/blender/makesrna/intern/rna_access.c
source/blender/python/intern/bpy_rna_anim.c

index 9cdb35586ced8e0c61a621591f1c92b3879166fb..48d83652f562653514f5657bcc8136ffa82b8cfa 100644 (file)
@@ -550,7 +550,7 @@ void BKE_animdata_separate_by_basepath(ID *srcID, ID *dstID, ListBase *basepaths
 /* Path Validation -------------------------------------------- */
 
 /* Check if a given RNA Path is valid, by tracing it from the given ID, and seeing if we can resolve it */
-static short check_rna_path_is_valid(ID *owner_id, const char *path)
+static bool check_rna_path_is_valid(ID *owner_id, const char *path)
 {
        PointerRNA id_ptr, ptr;
        PropertyRNA *prop = NULL;
@@ -559,7 +559,7 @@ static short check_rna_path_is_valid(ID *owner_id, const char *path)
        RNA_id_pointer_create(owner_id, &id_ptr);
        
        /* try to resolve */
-       return RNA_path_resolve(&id_ptr, path, &ptr, &prop); 
+       return RNA_path_resolve_property(&id_ptr, path, &ptr, &prop); 
 }
 
 /* Check if some given RNA Path needs fixing - free the given path and set a new one as appropriate 
@@ -1164,7 +1164,7 @@ static short animsys_write_rna_setting(PointerRNA *ptr, char *path, int array_in
        //printf("%p %s %i %f\n", ptr, path, array_index, value);
        
        /* get property to write to */
-       if (RNA_path_resolve(ptr, path, &new_ptr, &prop)) {
+       if (RNA_path_resolve_property(ptr, path, &new_ptr, &prop)) {
                /* set value - only for animatable numerical values */
                if (RNA_property_animateable(&new_ptr, prop)) {
                        int array_len = RNA_property_array_length(&new_ptr, prop);
@@ -1650,7 +1650,7 @@ static NlaEvalChannel *nlaevalchan_verify(PointerRNA *ptr, ListBase *channels, N
        /* free_path = */ /* UNUSED */ animsys_remap_path(strip->remap, fcu->rna_path, &path);
        
        /* a valid property must be available, and it must be animatable */
-       if (RNA_path_resolve(ptr, path, &new_ptr, &prop) == 0) {
+       if (RNA_path_resolve_property(ptr, path, &new_ptr, &prop) == false) {
                if (G.debug & G_DEBUG) printf("NLA Strip Eval: Cannot resolve path\n");
                return NULL;
        }
index 3141d52e22a5e3233bb19564fca7a74ea7dbc32d..c86ee11985a73e51fa02264c372fbe63cfea6087 100644 (file)
@@ -1040,7 +1040,7 @@ static float dtar_get_prop_val(ChannelDriver *driver, DriverTarget *dtar)
        RNA_id_pointer_create(id, &id_ptr);
        
        /* get property to read from, and get value as appropriate */
-       if (RNA_path_resolve_full(&id_ptr, dtar->rna_path, &ptr, &prop, &index)) {
+       if (RNA_path_resolve_property_full(&id_ptr, dtar->rna_path, &ptr, &prop, &index)) {
                if (RNA_property_array_check(prop)) {
                        /* array */
                        if ((index >= 0) && (index < RNA_property_array_length(&ptr, prop))) {
index bd580ab590f7c685aaff65213cb4b79d0f238cec..430e7f719b97f3904ace0f73dcf63fa635d2a062 100644 (file)
@@ -3280,7 +3280,7 @@ static void achannel_setting_slider_cb(bContext *C, void *id_poin, void *fcu_poi
        RNA_id_pointer_create(id, &id_ptr);
        
        /* try to resolve the path stored in the F-Curve */
-       if (RNA_path_resolve(&id_ptr, fcu->rna_path, &ptr, &prop)) {
+       if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) {
                /* set the special 'replace' flag if on a keyframe */
                if (fcurve_frame_has_keyframe(fcu, cfra, 0))
                        flag |= INSERTKEY_REPLACE;
@@ -3318,7 +3318,7 @@ static void achannel_setting_slider_shapekey_cb(bContext *C, void *key_poin, voi
        RNA_id_pointer_create((ID *)key, &id_ptr);
        
        /* try to resolve the path stored in the F-Curve */
-       if (RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop)) {
                /* find or create new F-Curve */
                // XXX is the group name for this ok?
                bAction *act = verify_adt_action((ID *)key, 1);
@@ -3615,7 +3615,7 @@ void ANIM_channel_draw_widgets(bContext *C, bAnimContext *ac, bAnimListElem *ale
                                        RNA_id_pointer_create(ale->id, &id_ptr);
                                        
                                        /* try to resolve the path */
-                                       if (RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop)) {
+                                       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop)) {
                                                uiBut *but;
                                                
                                                /* create the slider button, and assign relevant callback to ensure keyframes are inserted... */
index 98071fafc1e783062b5ce99ae0c872b7c3e457b0..618c1b04a71f042bcda784b8a3197885e8c53406 100644 (file)
@@ -87,7 +87,7 @@ void ANIM_list_elem_update(Scene *scene, bAnimListElem *ale)
                
                RNA_id_pointer_create(id, &id_ptr);
                        
-               if (RNA_path_resolve(&id_ptr, fcu->rna_path, &ptr, &prop))
+               if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop))
                        RNA_property_update_main(G.main, scene, &ptr, prop);
        }
        else {
index eb1f5ef1043cb0f53ee2fd56477827fa8e8b9165..bd4c234781534c76293bdce34d1ba20a5c42522a 100644 (file)
@@ -373,7 +373,7 @@ float ANIM_unit_mapping_get_factor(Scene *scene, ID *id, FCurve *fcu, short rest
                
                /* get RNA property that F-Curve affects */
                RNA_id_pointer_create(id, &id_ptr);
-               if (RNA_path_resolve(&id_ptr, fcu->rna_path, &ptr, &prop)) {
+               if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) {
                        /* rotations: radians <-> degrees? */
                        if (RNA_SUBTYPE_UNIT(RNA_property_subtype(prop)) == PROP_UNIT_ROTATION) {
                                /* if the radians flag is not set, default to using degrees which need conversions */
index 2352cad0cbc161b7d3948690271849ea9fef8ee8..21941c7ed625c557d6889d8d9fd0d8711097ed4c 100644 (file)
@@ -79,7 +79,7 @@ int getname_anim_fcurve(char *name, ID *id, FCurve *fcu)
                RNA_id_pointer_create(id, &id_ptr);
                
                /* try to resolve the path */
-               if (RNA_path_resolve(&id_ptr, fcu->rna_path, &ptr, &prop)) {
+               if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) {
                        const char *structname = NULL, *propname = NULL;
                        char arrayindbuf[16];
                        const char *arrayname = NULL;
index cd5e873f40dc4902513f64bc897ec6cf99338f07..d9965ebdcacc16db18d0f203794ba4e2f5720682 100644 (file)
@@ -145,7 +145,7 @@ short ANIM_add_driver(ReportList *reports, ID *id, const char rna_path[], int ar
        
        /* validate pointer first - exit if failure */
        RNA_id_pointer_create(id, &id_ptr);
-       if ((RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop) == 0) || (prop == NULL)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
                BKE_reportf(reports, RPT_ERROR, 
                            "Could not add driver, as RNA path is invalid for the given ID (ID = %s, path = %s)",
                            id->name, rna_path);
@@ -308,7 +308,7 @@ short ANIM_copy_driver(ReportList *reports, ID *id, const char rna_path[], int a
        
        /* validate pointer first - exit if failure */
        RNA_id_pointer_create(id, &id_ptr);
-       if ((RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop) == 0) || (prop == NULL)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
                BKE_reportf(reports, RPT_ERROR,
                            "Could not find driver to copy, as RNA path is invalid for the given ID (ID = %s, path = %s)",
                            id->name, rna_path);
@@ -355,7 +355,7 @@ short ANIM_paste_driver(ReportList *reports, ID *id, const char rna_path[], int
        
        /* validate pointer first - exit if failure */
        RNA_id_pointer_create(id, &id_ptr);
-       if ((RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop) == 0) || (prop == NULL)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
                BKE_reportf(reports, RPT_ERROR,
                            "Could not paste driver, as RNA path is invalid for the given ID (ID = %s, path = %s)",
                            id->name, rna_path);
index bc20311f773162492655383434e0d8b062af2bb0..d1df93f49f50c91f6afba0712c0ff7735076b44b 100644 (file)
@@ -810,7 +810,7 @@ short insert_keyframe_direct(ReportList *reports, PointerRNA ptr, PropertyRNA *p
                PointerRNA tmp_ptr;
                
                /* try to get property we should be affecting */
-               if ((RNA_path_resolve(&ptr, fcu->rna_path, &tmp_ptr, &prop) == 0) || (prop == NULL)) {
+               if (RNA_path_resolve_property(&ptr, fcu->rna_path, &tmp_ptr, &prop) == false) {
                        /* property not found... */
                        const char *idname = (ptr.id.data) ? ((ID *)ptr.id.data)->name : TIP_("<No ID pointer>");
                        
@@ -920,7 +920,7 @@ short insert_keyframe(ReportList *reports, ID *id, bAction *act, const char grou
        }
        
        RNA_id_pointer_create(id, &id_ptr);
-       if ((RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop) == 0) || (prop == NULL)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
                BKE_reportf(reports, RPT_ERROR,
                            "Could not insert keyframe, as RNA path is invalid for the given ID (ID = %s, path = %s)",
                            (id) ? id->name : TIP_("<Missing ID block>"), rna_path);
@@ -1012,7 +1012,7 @@ short delete_keyframe(ReportList *reports, ID *id, bAction *act, const char grou
        
        /* validate pointer first - exit if failure */
        RNA_id_pointer_create(id, &id_ptr);
-       if ((RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop) == 0) || (prop == NULL)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
                BKE_reportf(reports, RPT_ERROR,
                            "Could not delete keyframe, as RNA path is invalid for the given ID (ID = %s, path = %s)",
                            id->name, rna_path);
@@ -1113,7 +1113,7 @@ static short clear_keyframe(ReportList *reports, ID *id, bAction *act, const cha
 
        /* validate pointer first - exit if failure */
        RNA_id_pointer_create(id, &id_ptr);
-       if ((RNA_path_resolve(&id_ptr, rna_path, &ptr, &prop) == 0) || (prop == NULL)) {
+       if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
                BKE_reportf(reports, RPT_ERROR,
                            "Could not clear keyframe, as RNA path is invalid for the given ID (ID = %s, path = %s)",
                            id->name, rna_path);
index ad09fcb596658972bc797a93756a177b53620c86..07825f59c2f945d884f02887563019d1cbffcd18 100644 (file)
@@ -977,7 +977,7 @@ int ANIM_apply_keyingset(bContext *C, ListBase *dsources, bAction *act, KeyingSe
                        PropertyRNA *prop;
                        
                        RNA_id_pointer_create(ksp->id, &id_ptr);
-                       if (RNA_path_resolve(&id_ptr, ksp->rna_path, &ptr, &prop) && prop)
+                       if (RNA_path_resolve_property(&id_ptr, ksp->rna_path, &ptr, &prop))
                                arraylen = RNA_property_array_length(&ptr, prop);
                }
                
index ac01cbb5f4a64ca476e1bc01de86754783e7d790..0ff0e0d498ce38058c7d1ff4c1067e404abd1c25 100644 (file)
@@ -1027,7 +1027,7 @@ static short pose_propagate_get_refVal(Object *ob, FCurve *fcu, float *value)
        RNA_id_pointer_create(&ob->id, &id_ptr);
        
        /* resolve the property... */
-       if (RNA_path_resolve(&id_ptr, fcu->rna_path, &ptr, &prop)) {
+       if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) {
                if (RNA_property_array_check(prop)) {
                        /* array */
                        if (fcu->array_index < RNA_property_array_length(&ptr, prop)) {
index 52a26f4f5288f6b7e4593d2b9c60df655a43536b..e4b76cb039929ef4548d5e09f2e711a0eeef0878 100644 (file)
@@ -551,7 +551,7 @@ static int copy_to_selected_button_poll(bContext *C)
                                        if (use_path) {
                                                lprop = NULL;
                                                RNA_id_pointer_create(link->ptr.id.data, &idptr);
-                                               RNA_path_resolve(&idptr, path, &lptr, &lprop);
+                                               RNA_path_resolve_property(&idptr, path, &lptr, &lprop);
                                        }
                                        else {
                                                lptr = link->ptr;
@@ -601,7 +601,7 @@ static int copy_to_selected_button_exec(bContext *C, wmOperator *op)
                                        if (use_path) {
                                                lprop = NULL;
                                                RNA_id_pointer_create(link->ptr.id.data, &idptr);
-                                               RNA_path_resolve(&idptr, path, &lptr, &lprop);
+                                               RNA_path_resolve_property(&idptr, path, &lptr, &lprop);
                                        }
                                        else {
                                                lptr = link->ptr;
index f7ec8e8682dbe01c41c2a8ff2d9b43df10fc8a1e..d62ab2850b416353a5ea97f57fcf71712f0491d9 100644 (file)
@@ -296,7 +296,7 @@ static void graph_panel_key_properties(const bContext *C, Panel *pa)
                
                /* get property that F-Curve affects, for some unit-conversion magic */
                RNA_id_pointer_create(ale->id, &id_ptr);
-               if (RNA_path_resolve(&id_ptr, fcu->rna_path, &fcu_prop_ptr, &fcu_prop) && fcu_prop) {
+               if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &fcu_prop_ptr, &fcu_prop)) {
                        /* determine the unit for this property */
                        unit = RNA_SUBTYPE_UNIT(RNA_property_subtype(fcu_prop));
                }
index 7d817f3dbe01e5cde1423d24a3243f284da3ab91..a61a9b3da854dea148a1af125699ba20363d0797 100644 (file)
@@ -898,10 +898,18 @@ char *RNA_path_append(const char *path, PointerRNA *ptr, PropertyRNA *prop,
                       int intkey, const char *strkey);
 char *RNA_path_back(const char *path);
 
-int RNA_path_resolve(PointerRNA *ptr, const char *path,
+/* path_resolve() variants only ensure that a valid pointer (and optionally property) exist */
+bool RNA_path_resolve(PointerRNA *ptr, const char *path,
                      PointerRNA *r_ptr, PropertyRNA **r_prop);
 
-int RNA_path_resolve_full(PointerRNA *ptr, const char *path,
+bool RNA_path_resolve_full(PointerRNA *ptr, const char *path,
+                          PointerRNA *r_ptr, PropertyRNA **r_prop, int *index);
+                                                 
+/* path_resolve_property() variants ensure that pointer + property both exist */
+bool RNA_path_resolve_property(PointerRNA *ptr, const char *path,
+                     PointerRNA *r_ptr, PropertyRNA **r_prop);
+
+bool RNA_path_resolve_property_full(PointerRNA *ptr, const char *path,
                           PointerRNA *r_ptr, PropertyRNA **r_prop, int *index);
 
 char *RNA_path_from_ID_to_struct(PointerRNA *ptr);
index 9f36131a6c9e60893a7c074b6cf44c2ff69c1f60..9a102c823a9196ed853742849f22a42b675bf038 100644 (file)
@@ -630,7 +630,7 @@ PropertyRNA *RNA_struct_find_property(PointerRNA *ptr, const char *identifier)
                /* id prop lookup, not so common */
                PropertyRNA *r_prop = NULL;
                PointerRNA r_ptr; /* only support single level props */
-               if (RNA_path_resolve(ptr, identifier, &r_ptr, &r_prop) && r_ptr.type == ptr->type && r_ptr.data == ptr->data)
+               if (RNA_path_resolve(ptr, identifier, &r_ptr, &r_prop) && (r_ptr.type == ptr->type) && (r_ptr.data == ptr->data))
                        return r_prop;
        }
        else {
@@ -1533,7 +1533,7 @@ bool RNA_property_path_from_ID_check(PointerRNA *ptr, PropertyRNA *prop)
                PropertyRNA *r_prop;
 
                RNA_id_pointer_create(ptr->id.data, &id_ptr);
-               if (RNA_path_resolve(&id_ptr, path, &r_ptr, &r_prop) == TRUE) {
+               if (RNA_path_resolve(&id_ptr, path, &r_ptr, &r_prop) == true) {
                        ret = (prop == r_prop);
                }
                MEM_freeN(path);
@@ -3791,13 +3791,40 @@ static int rna_token_strip_quotes(char *token)
        return 0;
 }
 
-/* Resolve the given RNA path to find the pointer+property indicated at the end of the path */
-int RNA_path_resolve(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop)
+/* Resolve the given RNA Path to find both the pointer AND property indicated by fully resolving the path
+ * ! This is a convenience method to avoid logic errors and ugly syntax
+ * ! Assumes all pointers provided are valid
+ * > returns: True only if both a valid pointer and property are found after resolving the path 
+ */
+bool RNA_path_resolve_property(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop)
+{
+       return RNA_path_resolve_full(ptr, path, r_ptr, r_prop, NULL) && (*r_prop != NULL);
+}
+
+/* Resolve the given RNA Path to find the pointer AND property (as well as the array index) indicated by fully resolving the path
+ * ! This is a convenience method to avoid logic errors and ugly syntax
+ * ! Assumes all pointers provided are valid
+ * > returns: True only if both a valid pointer and property are found after resolving the path
+ */
+bool RNA_path_resolve_property_full(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop, int *index)
+{
+       return RNA_path_resolve_full(ptr, path, r_ptr, r_prop, index) && (*r_prop != NULL);
+}
+
+/* Resolve the given RNA Path to find the pointer and/or property indicated by fully resolving the path 
+ * ! Assumes all pointers provided are valid
+ * > returns: True if path can be resolved to a valid "pointer + property" OR "pointer only"
+ */
+bool RNA_path_resolve(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop)
 {
        return RNA_path_resolve_full(ptr, path, r_ptr, r_prop, NULL);
 }
 
-int RNA_path_resolve_full(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop, int *index)
+/* Resolve the given RNA Path to find the pointer and/or property + array index indicated by fully resolving the path 
+ * ! Assumes all pointers provided are valid
+ * > returns: True if path can be resolved to a valid "pointer + property" OR "pointer only"
+ */
+bool RNA_path_resolve_full(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop, int *index)
 {
        PropertyRNA *prop;
        PointerRNA curptr;
index b61ed55da0660218c3add3b0cc0b67491ba279aa..6e78a543481687524c65d786c655e324e9d59089 100644 (file)
@@ -72,7 +72,7 @@ static int pyrna_struct_anim_args_parse(
        /* full paths can only be given from ID base */
        if (is_idbase) {
                int r_index = -1;
-               if (RNA_path_resolve_full(ptr, path, &r_ptr, &prop, &r_index) == 0) {
+               if (RNA_path_resolve_property_full(ptr, path, &r_ptr, &prop, &r_index) == false) {
                        prop = NULL;
                }
                else if (r_index != -1) {