Fix T89435: Reordering FCurves can cause crash or corruption

Correctly reset `prev` and `next` pointers of action group FCurves when
separating them into distinct `ListBase`s per `bActionGroup`.

These `NULL` pointers are necessary to temporarily demarcate the start &
end of the `bActionGroup::channels` list. Having them still point to
other FCurves caused ordering issues when moving curves towards the
start/end of a group.

This commit corrects the above issue and adds versioning code to rectify
any ordering issues that may have been caused. For this purpose the
`BKE_action_groups_reconstruct()` function is rewritten to avoid relying
on the `bAction::curves` list order or `prev` link integrity.

Differential Revision: https://developer.blender.org/D11811

1 files changed, 3 insertions, 12 deletions

diff --git a/source/blender/editors/animation/anim_channels_edit.c b/source/blender/editors/animation/anim_channels_edit.c
index 76b6ce54d29..8f8c1c067d4 100644
--- a/source/blender/editors/animation/anim_channels_edit.c
+++ b/source/blender/editors/animation/anim_channels_edit.c
@@ -1283,6 +1283,9 @@ static void split_groups_action_temp(bAction *act, bActionGroup *tgrp)
     else {
       group_fcurves_last->next->prev = group_fcurves_first->prev;
     }
+
+    /* Clear links pointing outside the per-group list. */
+    group_fcurves_first->prev = group_fcurves_last->next = NULL;
   }
 
   /* Initialize memory for temp-group */
@@ -1337,24 +1340,12 @@ static void join_groups_action_temp(bAction *act)
     if (agrp->flag & AGRP_TEMP) {
       LISTBASE_FOREACH (FCurve *, fcu, &agrp->channels) {
         fcu->grp = NULL;
-        if (fcu == agrp->channels.last) {
-          break;
-        }
       }
 
       BLI_remlink(&act->groups, agrp);
       break;
     }
   }
-
-  /* BLI_movelisttolist() doesn't touch first->prev and last->next pointers in its "dst" list.
-   * Ensure that after the reshuffling the list is properly terminated. */
-  if (!BLI_listbase_is_empty(&act->curves)) {
-    FCurve *act_fcurves_first = act->curves.first;
-    act_fcurves_first->prev = NULL;
-    FCurve *act_fcurves_last = act->curves.last;
-    act_fcurves_last->next = NULL;
-  }
 }
 
 /* Change the order of anim-channels within action