Hi,
a friend brought to my attention, that dumpOSC doesn't handle lists with more than 50 elements properly. It simply crahes Pd. Attached patch shows this. (pd 0.39 required for "list").
The reason actually is quite simple: dumpOSC.c defines a maximum of atoms to hold in x->x_outat of being 50 as default, but later doesn't check at all, if this space is exceeded. Here's the relevant part of the struct declaration:
/* ----------------------------- dumpOSC ------------------------- */
#define MAXOUTAT 20
static t_class *dumpOSC_class;
typedef struct _dumpOSC { t_object x_obj; t_outlet *x_msgout; t_outlet *x_connectout; t_atom x_outat[MAXOUTAT]; int x_outatc; /* ... */ } t_dumpOSC;
Then later in the functions "dumpOSC_PrintTypeTaggedArgs" and "dumpOSC_PrintHeuristicallyTypeGuessedArgs" x->x_outat is happily filled with atoms as long as the incoming message list lasts, which of course is a recipe for desaster.
Now with my limited experience in C coding, I wonder, what's the best way to fix this.
I already wrapped all tries to append to x->x_outat in a big "if" clause which checks, if MAXOUTAT is reached. This works and keeps Pd from crashing. However somehow it doesn't feel right to just truncate the incoming message, even when posting a big warning message. OTOH I suppose, that declaring *x_outat with a fixed size was done for speed reasons. Having to allocate memory everytime a message comes in would be very slow.
Maybe one could introduce an argument to dumpOSC which sets the maximum allowed message length (with a default of 50)?
So what to do about this? Any advice or opinions? (Apart from using liblo and totally rewriting OSCx, which of course probably is the best solution ...)
Ciao
Hallo, Frank Barknecht hat gesagt: // Frank Barknecht wrote:
Here's the relevant part of the struct declaration:
/* ----------------------------- dumpOSC ------------------------- */
#define MAXOUTAT 20
Sorry, this is "#define MAXOUTAT 50" actually, I copied this smaller number from my changed source, where I wanted the crash with a shorter message to have shorter debug printouts ... :(
Ciao
Frank Barknecht wrote:
The reason actually is quite simple: dumpOSC.c defines a maximum of atoms to hold in x->x_outat of being 50 as default, but later doesn't check at all, if this space is exceeded. Here's the relevant part of the struct declaration:
/* ----------------------------- dumpOSC ------------------------- */
#define MAXOUTAT 20
static t_class *dumpOSC_class;
typedef struct _dumpOSC { t_object x_obj; t_outlet *x_msgout; t_outlet *x_connectout; t_atom x_outat[MAXOUTAT]; int x_outatc; /* ... */ } t_dumpOSC;
Then later in the functions "dumpOSC_PrintTypeTaggedArgs" and "dumpOSC_PrintHeuristicallyTypeGuessedArgs" x->x_outat is happily filled with atoms as long as the incoming message list lasts, which of course is a recipe for desaster.
Now with my limited experience in C coding, I wonder, what's the best way to fix this.
I already wrapped all tries to append to x->x_outat in a big "if" clause which checks, if MAXOUTAT is reached. This works and keeps Pd from crashing. However somehow it doesn't feel right to just truncate the incoming message, even when posting a big warning message. OTOH I suppose, that declaring *x_outat with a fixed size was done for speed reasons. Having to allocate memory everytime a message comes in would be very slow.
But isn't the memory allocated only when the object is created and reused for each OSC message?
Maybe one could introduce an argument to dumpOSC which sets the maximum allowed message length (with a default of 50)?
From the OSC spec: " The underlying network that delivers an OSC packet is responsible for delivering both the contents and the size to the OSC application. An OSC packet can be naturally represented by a datagram by a network protocol such as UDP. In a stream-based protocol such as TCP, the stream should begin with an int32 giving the size of the first packet, followed by the contents of the first packet, followed by the size of the second packet, etc. " For UDP the data length field in the header is 16 bits and the header itself is 8 bytes, so up to 65528 bytes can be used for an OSC packet, and each element in the OSC packet is 4bytes long, the same size as a t_atom (not counting the path and typetags). So with MAXOUTAT 16382 there would be no problem and no need for a bounds check. 65528 bytes would be no bit memory hog for most people... But for TCP the message could be up to 4,294,967,295 bytes long :(
Martin
Hallo, Martin Peach hat gesagt: // Martin Peach wrote:
Frank Barknecht wrote:
I already wrapped all tries to append to x->x_outat in a big "if" clause which checks, if MAXOUTAT is reached. This works and keeps Pd from crashing. However somehow it doesn't feel right to just truncate the incoming message, even when posting a big warning message. OTOH I suppose, that declaring *x_outat with a fixed size was done for speed reasons. Having to allocate memory everytime a message comes in would be very slow.
But isn't the memory allocated only when the object is created and reused for each OSC message?
Yep, that's the way it is now: On object creation, space for 50 atoms is allocated and then reused - unfortunatly without a bounds check yet.
From the OSC spec: " The underlying network that delivers an OSC packet is responsible for delivering both the contents and the size to the OSC application.
That's what I thought to refer to with "allocate memory everytime a message comes in": We could get the size from the OSC packet, and then allocate enough atom space in the Pd object according to the packet length. However this would mean allocating and possibly freeing memory all the time, that is, with every incoming message. Or not?
For UDP the data length field in the header is 16 bits and the header itself is 8 bytes, so up to 65528 bytes can be used for an OSC packet, and each element in the OSC packet is 4bytes long, the same size as a t_atom (not counting the path and typetags). So with MAXOUTAT 16382 there would be no problem and no need for a bounds check. 65528 bytes would be no bit memory hog for most people...
Especially when you normally run only a handful of dumpOSC objects, often it will be only a single one.
But for TCP the message could be up to 4,294,967,295 bytes long :(
Urgs. ;)
Ciao
-
Frank Barknecht -
Martin Peach