[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Sc-devel] [BUG] bugs with n_setn and s_getn?



hi sc-devs,

while playing around with the setn/getn functionalities in sc (@linux)
i bumped into some problems which i tried to track down.  i don't know
whether i found the actual bugs or only symptoms and whether my fixes
are elegant enough and/or compliant to coding styles, but anyway, my
fixes are attached as a patch to the sc source tree.

1) Node.setn does not work with integer control indices

in Node.sc i found

args.pairsDo({ arg control, moreVals; 
	       nargs.addAll([control.asSymbol, moreVals.size, moreVals].flat)});

control.asSymbol makes a symbol out of every input, which makes it
impossible to get an integer argument into the osc message by this
method.

I had a temporary workaround which is too ugly to post it here, but
I talked to Julian who will find a solution for a .flat-message which
does not flatten a string.

2) Synth.getn crashes server when used with control symbols

server does not crash when using getn with control indices instead.

i tracked this down a bit and found something strange in
Headers/plugin_interface/sc_msg_iter.h

>>>>>>>>>

inline int32 sc_msg_iter::geti(int32 defaultValue)
{
        int value;
        if (remain() <= 0) return defaultValue;
        if (tags) {
                if (tags[count] == 'i') {
                        value = OSCint(rdpos);
                        rdpos += sizeof(int32);
                } else if (tags[count] == 'f') {
                        value = (int32)OSCfloat(rdpos);
                        rdpos += sizeof(float32);
/*              } else if (tags[count] == 's') {
                        value = atoi(rdpos);
                        rdpos = OSCstrskip(rdpos);
*/
                } else {
                        value = defaultValue;
                }
        } else {
                value = (int)OSCint(rdpos);
                rdpos += sizeof(int32);
        }
        count ++;
        return value;
}

<<<<<<<< schnipp


the check for a string tag which is commented out means that whenever
a string element in an osc message is skipped with e.g. geti(),
count++ gets incremented while the read pointer rdpos remains
untouched.  this leads to misinterpretation in subsequent parse
attempts of the message, as for example in parsing getn messages with
string control names following by an integer.

i don't know the reason why this was commented out nor when, in svn
even the earliest revisions seem already to contain this bug.  i guess
the intention was this one, but i am not sure:

		} else if (tags[count] == 's') {
		  /*	value = atoi(rdpos); */
		        value = defaultValue;
 			rdpos = OSCstrskip(rdpos);
 		} else {
		        /* this is dangerous, as rdpos is not
			   advanced accordingly while count++ takes
			   place */
 			value = defaultValue;
 		}

btw. the same problem applies to the default case, but don't know
when it applies.


3) get or getn osc messages using control symbol names instead of
numeric indices _sometimes_ yield apparently malformed reply messages

especially when asking for multiple values, e.g.

s.sendMsg("s_get", nodeId, \out, \in, \gain, \matrix);

s.sendMsg("s_getn", nodeId, \out, 1, \matrix, 16);


if i uderstand correctly, in Source/server/SC_MiscCmds.cpp the number
of requests in a s_get/n message is calculated by dividing the number
of remaining bytes in the incoming osc message by 4 (resp. by 8 for s_getn):

in SCErr meth_s_get(): int numheads = msg.remain() >> 2;

in SCErr meth_s_getn():	int numheads = msg.remain() >> 3;

i suspect this is true only for control index arguments, not for
strings >= 4 bytes (excl. 0-byte).  for many cases, this works fine
anyway.  this yields a malformed reply message only if it triggers
another 4-byte-word being reserved for the typetags which remains
entirely unused, causing a 4-byte-offset mismatch between the
typetags and the message elements' data.

i am not quite happy with my fix, as it is not the most performant one
and works only for typetagged messages, falls back to the old
behaviour with non-typetagged messages (do they exist anymore?):


in SCErr meth_s_get():
	int numheads = msg.tags ? strlen(msg.tags) - 1 : msg.remain() >> 2;

in SCErr meth_s_getn():
+	int numheads = msg.tags ? strlen(msg.tags) - 1 >> 1 : msg.remain() >> 3;


attached a patch which contains these fixes, applies against sc-3.1
source.

all the best,

martin
diff -rpuN supercollider-orig/Headers/plugin_interface/sc_msg_iter.h supercollider/Headers/plugin_interface/sc_msg_iter.h
--- supercollider-orig/Headers/plugin_interface/sc_msg_iter.h	2007-09-17 16:46:11.000000000 +0200
+++ supercollider/Headers/plugin_interface/sc_msg_iter.h	2007-10-20 21:21:25.000000000 +0200
@@ -122,11 +122,14 @@ inline int32 sc_msg_iter::geti(int32 def
 		} else if (tags[count] == 'f') {
 			value = (int32)OSCfloat(rdpos);
 			rdpos += sizeof(float32);
-/*		} else if (tags[count] == 's') {
-			value = atoi(rdpos);
+		} else if (tags[count] == 's') {
+		  /*	value = atoi(rdpos); */
+		        value = defaultValue;
 			rdpos = OSCstrskip(rdpos);
-*/
 		} else {
+		        /* this is dangerous, as rdpos is not
+			   advanced accordingly while count++ takes
+			   place */
 			value = defaultValue;
 		}
 	} else {
@@ -151,11 +154,14 @@ inline float32 sc_msg_iter::getf(float32
 		} else if (tags[count] == 'i') {
 			value = static_cast<float32>(OSCint(rdpos));
 			rdpos += sizeof(int32);
-/*		} else if (tags[count] == 's') {
-			value = atof(rdpos);
+		} else if (tags[count] == 's') {
+		  /*    value = atof(rdpos); */
+		        value = defaultValue;
 			rdpos = OSCstrskip(rdpos);
-*/
 		} else {
+		        /* this is dangerous, as rdpos is not
+			   advanced accordingly while count++ takes
+			   place */
 			value = defaultValue;
 		}
 	} else {
@@ -180,11 +186,14 @@ inline float64 sc_msg_iter::getd(float64
 		} else if (tags[count] == 'i') {
 			value = (float64)OSCint(rdpos);
 			rdpos += sizeof(int32);
-/*		} else if (tags[count] == 's') {
-			value = atof(rdpos);
+		} else if (tags[count] == 's') {
+		  /*    value = atof(rdpos); */
+			value = defaultValue;
 			rdpos = OSCstrskip(rdpos);
-*/
 		} else {
+		        /* this is dangerous, as rdpos is not
+			   advanced accordingly while count++ takes
+			   place */
 			value = defaultValue;
 		}
 	} else {
diff -rpuN supercollider-orig/Source/server/SC_MiscCmds.cpp supercollider/Source/server/SC_MiscCmds.cpp
--- supercollider-orig/Source/server/SC_MiscCmds.cpp	2007-10-20 13:59:09.000000000 +0200
+++ supercollider/Source/server/SC_MiscCmds.cpp	2007-10-20 21:21:05.000000000 +0200
@@ -1341,7 +1341,7 @@ SCErr meth_s_get(World *inWorld, int inS
 	Graph *graph = Msg_GetGraph(inWorld, msg);
 	if (!graph) return kSCErr_NodeNotFound;
 			
-	int numheads = msg.remain() >> 2;
+	int numheads = msg.tags ? strlen(msg.tags) - 1 : msg.remain() >> 2;
 
 	big_scpacket packet;
 	packet.adds("/n_set");
@@ -1388,7 +1388,7 @@ SCErr meth_s_getn(World *inWorld, int in
 	
 	// figure out how many tags to allocate   
 	int numcontrols = 0;
-	int numheads = msg.remain() >> 3;
+	int numheads = msg.tags ? strlen(msg.tags) - 1 >> 1 : msg.remain() >> 3;
 	
 	while (msg.remain()) {
 		msg.geti(); // skip start

Attachment: signature.asc
Description: Digital signature