BLI_string: return NULL from BLI_str_quoted_substrN on failure

This was returning an empty allocated string, however almost
all callers checked if the return value was NULL before freeing,
making for misunderstandings on the intended use of this function.

BCAnimationSampler::initialize_curves for example detected the
f-curves animation type to 'bone' based on a non-NULL return value
which never failed.

Also fixes two leaks where the the result of BLI_str_quoted_substrN
wasn't freed.

10 files changed, 56 insertions, 55 deletions

diff --git a/source/blender/blenlib/intern/string.c b/source/blender/blenlib/intern/string.c
index dccc700c761..b0d87838d06 100644
--- a/source/blender/blenlib/intern/string.c
+++ b/source/blender/blenlib/intern/string.c
@@ -445,7 +445,7 @@ char *BLI_str_quoted_substrN(const char *__restrict str, const char *__restrict
       return result;
     }
   }
-  return BLI_strdupn("", 0);
+  return NULL;
 }
 
 /**
diff --git a/source/blender/editors/animation/anim_deps.c b/source/blender/editors/animation/anim_deps.c
index e552a321bca..32ce78e405e 100644
--- a/source/blender/editors/animation/anim_deps.c
+++ b/source/blender/editors/animation/anim_deps.c
@@ -216,11 +216,13 @@ static void animchan_sync_fcurve_scene(bAnimListElem *ale)
 
   /* get strip name, and check if this strip is selected */
   char *seq_name = BLI_str_quoted_substrN(fcu->rna_path, "sequences_all[");
-  Sequence *seq = BKE_sequence_get_by_name(ed->seqbasep, seq_name, false);
-  if (seq_name) {
-    MEM_freeN(seq_name);
+  if (seq_name == NULL) {
+    return;
   }
 
+  Sequence *seq = BKE_sequence_get_by_name(ed->seqbasep, seq_name, false);
+  MEM_freeN(seq_name);
+
   if (seq == NULL) {
     return;
   }
diff --git a/source/blender/editors/animation/anim_filter.c b/source/blender/editors/animation/anim_filter.c
index 2e65fff69f1..f4a487140ff 100644
--- a/source/blender/editors/animation/anim_filter.c
+++ b/source/blender/editors/animation/anim_filter.c
@@ -1063,13 +1063,12 @@ static bool skip_fcurve_selected_data(bDopeSheet *ads, FCurve *fcu, ID *owner_id
 
     /* only consider if F-Curve involves pose.bones */
     if ((fcu->rna_path) && strstr(fcu->rna_path, "pose.bones")) {
-      bPoseChannel *pchan;
-      char *bone_name;
 
       /* get bone-name, and check if this bone is selected */
-      bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
-      pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
+      bPoseChannel *pchan = NULL;
+      char *bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
       if (bone_name) {
+        pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
         MEM_freeN(bone_name);
       }
 
@@ -1106,13 +1105,12 @@ static bool skip_fcurve_selected_data(bDopeSheet *ads, FCurve *fcu, ID *owner_id
     if ((fcu->rna_path) && strstr(fcu->rna_path, "sequences_all")) {
       Editing *ed = BKE_sequencer_editing_get(scene, false);
       Sequence *seq = NULL;
-      char *seq_name;
 
       if (ed) {
         /* get strip name, and check if this strip is selected */
-        seq_name = BLI_str_quoted_substrN(fcu->rna_path, "sequences_all[");
-        seq = BKE_sequence_get_by_name(ed->seqbasep, seq_name, false);
+        char *seq_name = BLI_str_quoted_substrN(fcu->rna_path, "sequences_all[");
         if (seq_name) {
+          seq = BKE_sequence_get_by_name(ed->seqbasep, seq_name, false);
           MEM_freeN(seq_name);
         }
       }
@@ -1130,13 +1128,12 @@ static bool skip_fcurve_selected_data(bDopeSheet *ads, FCurve *fcu, ID *owner_id
 
     /* check for selected nodes */
     if ((fcu->rna_path) && strstr(fcu->rna_path, "nodes")) {
-      bNode *node;
-      char *node_name;
+      bNode *node = NULL;
 
       /* get strip name, and check if this strip is selected */
-      node_name = BLI_str_quoted_substrN(fcu->rna_path, "nodes[");
-      node = nodeFindNodebyName(ntree, node_name);
+      char *node_name = BLI_str_quoted_substrN(fcu->rna_path, "nodes[");
       if (node_name) {
+        node = nodeFindNodebyName(ntree, node_name);
         MEM_freeN(node_name);
       }
 
diff --git a/source/blender/editors/animation/anim_ipo_utils.c b/source/blender/editors/animation/anim_ipo_utils.c
index 72103d68b05..5992545bdbe 100644
--- a/source/blender/editors/animation/anim_ipo_utils.c
+++ b/source/blender/editors/animation/anim_ipo_utils.c
@@ -112,7 +112,8 @@ int getname_anim_fcurve(char *name, ID *id, FCurve *fcu)
         char *constName = BLI_str_quoted_substrN(fcu->rna_path, "constraints[");
 
         /* assemble the string to display in the UI... */
-        structname = BLI_sprintfN("%s : %s", pchanName, constName);
+        structname = BLI_sprintfN(
+            "%s : %s", pchanName ? pchanName : "", constName ? constName : "");
         free_structname = 1;
 
         /* free the temp names */
diff --git a/source/blender/editors/animation/keyframes_general.c b/source/blender/editors/animation/keyframes_general.c
index 08b02020f76..6ed9803dbd3 100644
--- a/source/blender/editors/animation/keyframes_general.c
+++ b/source/blender/editors/animation/keyframes_general.c
@@ -767,16 +767,15 @@ short copy_animedit_keys(bAnimContext *ac, ListBase *anim_data)
      * Storing the relevant information here helps avoiding crashes if we undo-repaste. */
     if ((aci->id_type == ID_OB) && (((Object *)aci->id)->type == OB_ARMATURE) && aci->rna_path) {
       Object *ob = (Object *)aci->id;
-      bPoseChannel *pchan;
-      char *bone_name;
 
-      bone_name = BLI_str_quoted_substrN(aci->rna_path, "pose.bones[");
-      pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
-      if (pchan) {
-        aci->is_bone = true;
-      }
+      char *bone_name = BLI_str_quoted_substrN(aci->rna_path, "pose.bones[");
       if (bone_name) {
+        bPoseChannel *pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
         MEM_freeN(bone_name);
+
+        if (pchan) {
+          aci->is_bone = true;
+        }
       }
     }
 
diff --git a/source/blender/editors/animation/keyframing.c b/source/blender/editors/animation/keyframing.c
index 09c33c48170..e8146ca960a 100644
--- a/source/blender/editors/animation/keyframing.c
+++ b/source/blender/editors/animation/keyframing.c
@@ -2236,20 +2236,18 @@ static int clear_anim_v3d_exec(bContext *C, wmOperator *UNUSED(op))
         /* in pose mode, only delete the F-Curve if it belongs to a selected bone */
         if (ob->mode & OB_MODE_POSE) {
           if ((fcu->rna_path) && strstr(fcu->rna_path, "pose.bones[")) {
-            bPoseChannel *pchan;
-            char *bone_name;
 
             /* get bone-name, and check if this bone is selected */
-            bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
-            pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
+            char *bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
             if (bone_name) {
+              bPoseChannel *pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
               MEM_freeN(bone_name);
-            }
 
-            /* delete if bone is selected*/
-            if ((pchan) && (pchan->bone)) {
-              if (pchan->bone->flag & BONE_SELECTED) {
-                can_delete = true;
+              /* delete if bone is selected*/
+              if ((pchan) && (pchan->bone)) {
+                if (pchan->bone->flag & BONE_SELECTED) {
+                  can_delete = true;
+                }
               }
             }
           }
@@ -2342,13 +2340,12 @@ static int delete_key_v3d_exec(bContext *C, wmOperator *op)
          * In object mode, we're dealing with the entire object.
          */
         if ((ob->mode & OB_MODE_POSE) && strstr(fcu->rna_path, "pose.bones[\"")) {
-          bPoseChannel *pchan;
-          char *bone_name;
+          bPoseChannel *pchan = NULL;
 
           /* get bone-name, and check if this bone is selected */
-          bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
-          pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
+          char *bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
           if (bone_name) {
+            pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
             MEM_freeN(bone_name);
           }
 
diff --git a/source/blender/editors/armature/pose_select.c b/source/blender/editors/armature/pose_select.c
index 731d0d10e0b..6a03207b3b0 100644
--- a/source/blender/editors/armature/pose_select.c
+++ b/source/blender/editors/armature/pose_select.c
@@ -1090,6 +1090,7 @@ static bool pose_select_same_keyingset(bContext *C, ReportList *reports, bool ex
 
           if (boneName) {
             bPoseChannel *pchan = BKE_pose_channel_find_name(pose, boneName);
+            MEM_freeN(boneName);
 
             if (pchan) {
               /* select if bone is visible and can be affected */
@@ -1098,9 +1099,6 @@ static bool pose_select_same_keyingset(bContext *C, ReportList *reports, bool ex
                 changed = true;
               }
             }
-
-            /* free temp memory */
-            MEM_freeN(boneName);
           }
         }
       }
diff --git a/source/blender/editors/transform/transform_convert_armature.c b/source/blender/editors/transform/transform_convert_armature.c
index a7301161570..e9b2273b343 100644
--- a/source/blender/editors/transform/transform_convert_armature.c
+++ b/source/blender/editors/transform/transform_convert_armature.c
@@ -164,21 +164,21 @@ static void autokeyframe_pose(
           /* only if bone name matches too...
            * NOTE: this will do constraints too, but those are ok to do here too?
            */
-          if (pchanName && STREQ(pchanName, pchan->name)) {
-            insert_keyframe(bmain,
-                            reports,
-                            id,
-                            act,
-                            ((fcu->grp) ? (fcu->grp->name) : (NULL)),
-                            fcu->rna_path,
-                            fcu->array_index,
-                            &anim_eval_context,
-                            ts->keyframe_type,
-                            &nla_cache,
-                            flag);
-          }
-
           if (pchanName) {
+            if (STREQ(pchanName, pchan->name)) {
+              insert_keyframe(bmain,
+                              reports,
+                              id,
+                              act,
+                              ((fcu->grp) ? (fcu->grp->name) : (NULL)),
+                              fcu->rna_path,
+                              fcu->array_index,
+                              &anim_eval_context,
+                              ts->keyframe_type,
+                              &nla_cache,
+                              flag);
+            }
+
             MEM_freeN(pchanName);
           }
         }
diff --git a/source/blender/io/collada/BCAnimationCurve.cpp b/source/blender/io/collada/BCAnimationCurve.cpp
index 33eaf3376cd..5065accf554 100644
--- a/source/blender/io/collada/BCAnimationCurve.cpp
+++ b/source/blender/io/collada/BCAnimationCurve.cpp
@@ -173,8 +173,14 @@ std::string BCAnimationCurve::get_animation_name(Object *ob) const
         name = "";
       }
       else {
-        const char *boneName = BLI_str_quoted_substrN(fcurve->rna_path, "pose.bones[");
-        name = (boneName) ? id_name(ob) + "_" + std::string(boneName) : "";
+        char *boneName = BLI_str_quoted_substrN(fcurve->rna_path, "pose.bones[");
+        if (boneName) {
+          name = id_name(ob) + "_" + std::string(boneName);
+          MEM_freeN(boneName);
+        }
+        else {
+          name = "";
+        }
       }
     } break;
 
diff --git a/source/blender/io/collada/BCAnimationSampler.cpp b/source/blender/io/collada/BCAnimationSampler.cpp
index abc770ceef5..a6ee0b8bee6 100644
--- a/source/blender/io/collada/BCAnimationSampler.cpp
+++ b/source/blender/io/collada/BCAnimationSampler.cpp
@@ -448,6 +448,7 @@ void BCAnimationSampler::initialize_curves(BCAnimationCurveMap &curves, Object *
         char *boneName = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
         if (boneName) {
           object_type = BC_ANIMATION_TYPE_BONE;
+          MEM_freeN(boneName);
         }
       }