Instead of swapping node names which necessitates allocating a temporary before, memory freeing/allocation and lots of copying (especially since you use CNA_CopyText) you should be reordering the nodes themselves. This will not only have a lot less overhead but will also make your function more generic and therefore more easily reusable for other purposes.

Also if you want to make your sort function really reusable for sorting all kinds of exec doubly linked lists I suggest adding a callback parameter of some kind that is used to compare nodes rather than hardcoding the comparison in the sort function.

@trixie

FWIW there has been for some time a feature request in bugzilla for adding a sort function for exec style doubly linked lists to "utility.library".

If no one else gets round to implementing it first I might just take care of it myself.

@jabirulo

Here's my two cents to the discussion:

The outer 'while()' loop moves forward through the list, whereas the inner 'while()' loop moves backwards through the list for as long as necessary.
In worst case (full inverse) it will go backwards until it hits the limit of the list '(Prev == NULL)', in which case you better simply inverse the list:

void Reverse_List(struct List *L)
{
  struct Node *First = IExec->GetHead(L),
              *Next;
 
  while ((Next = IExec->GetSucc(First)) != NULL)
   {
    IExec->Remove(Next);
    IExec->AddHead(L, Next);
   }
}

void Sort_List(struct List *L)
{
  struct Node *Curr = IExec->GetHead(L);
 
  if (Curr != NULL)
   {
    struct Node *Next = IExec->GetSucc(Curr),
                *Prev;
 
    while (Next != NULL)
     {
      if ((int)IUtility->CallHook(cmphook, Curr, Next) > 0)
       {
        IExec->Remove(Curr);
        IExec->Insert(L, Curr, Next);
        Prev = IExec->GetPred(Next);
 
        while (
               (Prev != NULL) &&
               ((int)IUtility->CallHook(cmphook, Prev, Next) > 0)
              )
         {
          IExec->Remove(Prev);
          IExec->Insert(L, Prev, Next);
          Prev = IExec->GetPred(Next);
         }
       }
 
      Next = IExec->GetSucc(Curr);
     }
   }
}

Have fun!
Btw, don't forget to detach the list from the gadget before you meddle with its nodes and reattach it when you're done.

OldFart