Handle out-of-range Patch.order

One patch - init - has a category and id  which isn't in range. Others may.
This results in +/_ browser sometimes going out of range on
an array and segfaulting. So be defensive in our beyond-the-edge
checkes. Addresses surge-synthesizer#319


Former-commit-id: b53297d1b6873ebbb0e116a07d595d0edc7103e4 [formerly 01b0953]
Former-commit-id: 09dcc967cb087993bcc0fcd4cc673449a6b88fe5
Former-commit-id: 0e4397049cebe38f429edb0539ed4c7e2abb1b21

src/common/SurgeStorage.cpp

@@ -539,14 +539,22 @@ int SurgeStorage::getAdjacentWaveTable(int id, bool nextPrev)
    if (!n)
       return -1;
 
-   int order = wt_list[id].order;
-
-   if (nextPrev)
-      order = (order == (n - 1)) ? 0 : order + 1;
+   // See comment in SurgeSynthesizerIO::incrementPatch and #319
+   if( id < 0 || id > n-1 )
+   {
+       return wtOrdering[0];
+   }
    else
-      order = (order == 0) ? n - 1 : order - 1;
-
-   return wtOrdering[order];
+   {
+       int order = wt_list[id].order;
+
+       if (nextPrev)
+           order = (order >= (n - 1)) ? 0 : order + 1; // see comment in incrementPatch for that >= vs ==
+       else
+           order = (order <= 0) ? n - 1 : order - 1;
+
+       return wtOrdering[order];
+   }
 }
 
 void SurgeStorage::clipboard_copy(int type, int scene, int entry)

src/common/SurgeSynthesizerIO.cpp

@@ -48,22 +48,46 @@ void SurgeSynthesizer::incrementPatch(bool nextPrev)
    if (!n)
       return;
 
-   int order = storage.patch_list[patchid].order;
-   int category = storage.patch_list[patchid].category;
-
-   if (nextPrev) {
-      do {
-         order = (order == (n - 1)) ? 0 : order + 1;
-      } while (storage.patch_list[storage.patchOrdering[order]].category !=
-               category);
-   } else {
-      do {
-         order = (order == 0) ? n - 1 : order - 1;
-      } while (storage.patch_list[storage.patchOrdering[order]].category !=
-               category);
+   /*
+   ** Ideally we would never call this with an out
+   ** of range patchid, but the init case where we
+   ** have a non-loaded in memory patch proves that
+   ** false, as may some other cases. So add this
+   ** defensive approach. See #319
+   */
+   if( patchid < 0 || patchid > n-1 )
+   {
+       // Find patch 0 category 0 and select it
+       int ccid = storage.patchCategoryOrdering[0];
+
+       int target = n+1;
+       for(auto &patch : storage.patch_list)
+       {
+           if(patch.category == ccid && patch.order < target)
+               target = patch.order;
+       }
+
+       patchid_queue = storage.patchOrdering[target];
+       current_category_id = ccid;
+   }
+   else
+   {
+       int order = storage.patch_list[patchid].order;
+       int category = storage.patch_list[patchid].category;
+
+       if (nextPrev) {
+           do {
+               order = (order >= (n - 1)) ? 0 : order + 1;
+           } while (storage.patch_list[storage.patchOrdering[order]].category !=
+                    category);
+       } else {
+           do {
+               order = (order <= 0) ? n - 1 : order - 1;
+           } while (storage.patch_list[storage.patchOrdering[order]].category !=
+                    category);
+       }
+       patchid_queue = storage.patchOrdering[order];
    }
-
-   patchid_queue = storage.patchOrdering[order];
    processThreadunsafeOperations();
    return;
 }
@@ -74,22 +98,30 @@ void SurgeSynthesizer::incrementCategory(bool nextPrev)
    if (!n)
       return;
 
-   int order = storage.patch_category[current_category_id].order;
-   if (nextPrev)
-      order = (order == (n - 1)) ? 0 : order + 1;
+   // See comment above and #319
+   if(current_category_id < 0 || current_category_id > n-1 )
+   {
+       current_category_id = storage.patchCategoryOrdering[0];
+   }
    else
-      order = (order == 0) ? n - 1 : order - 1;
-
-   current_category_id = storage.patchCategoryOrdering[order];
-
+   {
+       int order = storage.patch_category[current_category_id].order;
+       if (nextPrev)
+           order = (order >= (n - 1)) ? 0 : order + 1;
+       else
+           order = (order <= 0) ? n - 1 : order - 1;
+
+       current_category_id = storage.patchCategoryOrdering[order];
+   }
+
    // Find the first patch within the category.
    for (auto p : storage.patchOrdering)
    {
-      if (storage.patch_list[p].category == current_category_id)
-      {
-         patchid_queue = p;
-         processThreadunsafeOperations();
-         return;
+       if (storage.patch_list[p].category == current_category_id)
+       {
+           patchid_queue = p;
+           processThreadunsafeOperations();
+           return;
       }
    }
 }