From 01b0953c1a933f2693dd5720e1243e6b53eb1500 Mon Sep 17 00:00:00 2001
From: Paul Walker <paul@pwjw.com>
Date: Thu, 17 Jan 2019 08:38:51 -0500
Subject: [PATCH] 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 #319
---
 src/common/SurgeStorage.cpp       | 22 +++++---
 src/common/SurgeSynthesizerIO.cpp | 86 +++++++++++++++++++++----------
 2 files changed, 74 insertions(+), 34 deletions(-)

diff --git a/src/common/SurgeStorage.cpp b/src/common/SurgeStorage.cpp
index 313486a91ac..1dbb7fd839e 100644
--- a/src/common/SurgeStorage.cpp
+++ b/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)
diff --git a/src/common/SurgeSynthesizerIO.cpp b/src/common/SurgeSynthesizerIO.cpp
index 8230592d9af..c2e0fc323ca 100644
--- a/src/common/SurgeSynthesizerIO.cpp
+++ b/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;
       }
    }
 }