From 79b614f993f621166c8219ee35016152ae49b9fa Mon Sep 17 00:00:00 2001
From: Albert Graef <aggraef@gmail.com>
Date: Tue, 22 Jun 2021 15:33:20 +0200
Subject: [PATCH] mrpeach/unpackOSC: Refactored for better efficiency, fix
stack overflows on Windows.
---
externals/mrpeach/osc/unpackOSC.c | 94 ++++++++++++++++++++-----------
1 file changed, 60 insertions(+), 34 deletions(-)
diff --git a/externals/mrpeach/osc/unpackOSC.c b/externals/mrpeach/osc/unpackOSC.c
index 54b8b9b97..f5330202f 100644
--- a/externals/mrpeach/osc/unpackOSC.c
+++ b/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--;
}
--
GitLab