mrpeach/unpackOSC: Refactored for better efficiency, fix stack overflows on Windows.

externals/mrpeach/osc/unpackOSC.c

@@ -64,7 +64,7 @@ The OSC webpage is http://cnmat.cnmat.berkeley.edu/OpenSoundControl
         on OSX anyway.
 
 */
-//define DEBUG
+//#define DEBUG
 #include "packingOSC.h"
 
 
@@ -118,31 +118,27 @@ void unpackOSC_setup(void)
 }
 
 /* unpackOSC_list expects an OSC packet in the form of a list of floats on [0..255] */
+static void unpackOSC_worker(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv, char *buf);
 static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv) 
 {
-    int             size, messageLen, i, j;
-    char            *messageName, *args, *buf;
-    OSCTimeTag      tt;
-    t_atom          data_at[MAX_MESG] ={{0}};/* symbols making up the path + payload */
-    int             data_atc = 0;/* number of symbols to be output */
-    char            raw[MAX_MESG];/* bytes making up the entire OSC message */
-    int             raw_c;/* number of bytes in OSC message */
-
-#ifdef DEBUG
-    printf(">>> unpackOSC_list: %d bytes, abort=%d, reentry_count %d recursion_level %d\n",
-        argc, x->x_abort_bundle, x->x_reentry_count, x->x_recursion_level);
-#endif
-    if(x->x_abort_bundle) return; /* if backing quietly out of the recursive stack */
-    x->x_reentry_count++;
+    // ag: This is a small wrapper around unpackOSC_worker below which does all the real
+    // work. It does some preliminary checks and then sets up a buffer populated with the
+    // binary data of the message which then gets passed to the worker. That way we don't have
+    // to reparse the data over and over again in each recursive call to the worker, as was
+    // done in the original implementation, wasting a lot of time and stack space.
+    int             i, j;
+    // This can be static as it's only read by the worker, never modified.
+    static char     raw[MAX_MESG];/* bytes making up the entire OSC message */
+    // preliminary checks
     if ((argc%4) != 0)
     {
         post("unpackOSC: Packet size (%d) not a multiple of 4 bytes: dropping packet", argc);
-        goto unpackOSC_list_out;
+        return;
     }
     if(argc > MAX_MESG)
     {
         post("unpackOSC: Packet size (%d) greater than max (%d). Change MAX_MESG and recompile if you want more.", argc, MAX_MESG);
-        goto unpackOSC_list_out;
+        return;
     }
     /* copy the list to a byte buffer, checking for bytes only */
     for (i = 0; i < argc; ++i)
@@ -160,17 +156,47 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
             else
             {
                 post("unpackOSC: Data out of range (%d), dropping packet", argv[i].a_w.w_float);
-                goto unpackOSC_list_out;
+                return;
             }
         }
         else
         {
             post("unpackOSC: Data not float, dropping packet");
-            goto unpackOSC_list_out;
+            return;
         }
     }
-    raw_c = argc;
-    buf = raw;
+    unpackOSC_worker(x, s, argc, argv, raw);
+}
+
+static void unpackOSC_worker(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv, char *buf) 
+{
+    int             size, messageLen, i;
+    char            *messageName, *args;
+    OSCTimeTag      tt;
+    // ag: The original implementation reserved an excessive amount of storage on the stack
+    // here which caused the object to crash with a C stack overflow on Windows. We use a more
+    // reasonable limit here (as each element of an OSC message must consist of at least 4 bytes,
+    // MAX_MESG/4 should always be big enough), and we can also allocate the memory in static
+    // storage, since it is only used in the non-recursive part of the function.
+    static t_atom   data_at[MAX_MESG/4] ={{0}};/* symbols making up the path + payload */
+    int             data_atc = 0;/* number of symbols to be output */
+
+#ifdef DEBUG
+    printf(">>> unpackOSC_worker: %d bytes, abort=%d, reentry_count %d recursion_level %d\n",
+        argc, x->x_abort_bundle, x->x_reentry_count, x->x_recursion_level);
+#endif
+    if(x->x_abort_bundle) return; /* if backing quietly out of the recursive stack */
+    x->x_reentry_count++;
+    if ((argc%4) != 0)
+    {
+        post("unpackOSC: Packet size (%d) not a multiple of 4 bytes: dropping packet", argc);
+        goto unpackOSC_worker_out;
+    }
+    if(argc > MAX_MESG)
+    {
+        post("unpackOSC: Packet size (%d) greater than max (%d). Change MAX_MESG and recompile if you want more.", argc, MAX_MESG);
+        goto unpackOSC_worker_out;
+    }
 
     if ((argc >= 8) && (strncmp(buf, "#bundle", 8) == 0))
     { /* This is a bundle message. */
@@ -181,7 +207,7 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
         if (argc < 16)
         {
             post("unpackOSC: Bundle message too small (%d bytes) for time tag", argc);
-            goto unpackOSC_list_out;
+            goto unpackOSC_worker_out;
         }
 
         x->x_bundle_flag = 1;
@@ -207,13 +233,13 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
             if ((size % 4) != 0)
             {
                 post("unpackOSC: Bad size count %d in bundle (not a multiple of 4)", size);
-                goto unpackOSC_list_out;
+                goto unpackOSC_worker_out;
             }
             if ((size + i + 4) > argc)
             {
                 post("unpackOSC: Bad size count %d in bundle (only %d bytes left in entire bundle)",
                     size, argc-i-4);
-                goto unpackOSC_list_out;
+                goto unpackOSC_worker_out;
             }
 
             /* Recursively handle element of bundle */
@@ -225,13 +251,13 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
             {
                 post("unpackOSC: bundle depth %d exceeded", MAX_BUNDLE_NESTING);
                 x->x_abort_bundle = 1;/* we need to back out of the recursive stack*/
-                goto unpackOSC_list_out;
+                goto unpackOSC_worker_out;
             }
 #ifdef DEBUG
-            printf("unpackOSC: bundle calling unpackOSC_list(x=%p, s=%s, size=%d, argv[%d]=%p)\n",
+            printf("unpackOSC: bundle calling unpackOSC_worker(x=%p, s=%s, size=%d, argv[%d]=%p)\n",
               x, s->s_name, size, i+4, &argv[i+4]);
 #endif
-            unpackOSC_list(x, s, size, &argv[i+4]);
+            unpackOSC_worker(x, s, size, &argv[i+4], buf+i+4);
             i += 4 + size;
         }
 
@@ -249,20 +275,20 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
     else if ((argc == 24) && (strcmp(buf, "#time") == 0))
     {
         post("unpackOSC: Time message: %s\n :).\n", buf);
-        goto unpackOSC_list_out;
+        goto unpackOSC_worker_out;
     }
     else
     { /* This is not a bundle message or a time message */
 
         messageName = buf;
 #ifdef DEBUG
-        printf("unpackOSC: message name string: %s length %d\n", messageName, raw_c);
+        printf("unpackOSC: message name string: %s length %d\n", messageName, argc);
 #endif
-        args = unpackOSC_DataAfterAlignedString(messageName, buf+raw_c);
+        args = unpackOSC_DataAfterAlignedString(messageName, buf+argc);
         if (args == 0)
         {
             post("unpackOSC: Bad message name string: Dropping entire message.");
-            goto unpackOSC_list_out;
+            goto unpackOSC_worker_out;
         }
         messageLen = args-messageName;
         /* put the OSC path into a single symbol */
@@ -270,9 +296,9 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
         if (data_atc == 1)
         {
 #ifdef DEBUG
-            printf("unpackOSC_list calling unpackOSC_Smessage: message length %d\n", raw_c-messageLen);
+            printf("unpackOSC_worker calling unpackOSC_Smessage: message length %d\n", argc-messageLen);
 #endif
-            unpackOSC_Smessage(data_at, &data_atc, (void *)args, raw_c-messageLen);
+            unpackOSC_Smessage(data_at, &data_atc, (void *)args, argc-messageLen);
             if (0 == x->x_bundle_flag)
                 outlet_float(x->x_delay_out, 0); /* no delay for message not in a bundle */
         }
@@ -281,7 +307,7 @@ static void unpackOSC_list(t_unpackOSC *x, t_symbol *s, int argc, t_atom *argv)
         outlet_anything(x->x_data_out, atom_getsymbol(data_at), data_atc-1, data_at+1);
     data_atc = 0;
     x->x_abort_bundle = 0;
-unpackOSC_list_out:
+unpackOSC_worker_out:
     x->x_recursion_level = 0;
     x->x_reentry_count--;
 }