Menu

#8 Problematic sscanf patterns in config parsing code

1.0
closed
nobody
None
2015-06-01
2015-05-31
No

I think that these sscanf patterns used in the config parsing code are wrong or at least suspicious:

"%[ \t#]"

This is in main.c and supposed to match a comment line (presumably a line that begins with #), but will actually match any line that starts with # or whitespace. Is this intended?

"%s %*[^,],%*[^:]:%*[^(](%*[^)])"

This is supposed to match a config line, but it won't match lines for which the OSC message doesn't have any arguments, so that lines like the following will not be parsed correctly and give the error message argument type ':' not supported! instead:

/1 : noteon( 0, 60, 127 );

(TouchOSC actually generates these when switching tabs in the control interface.)

Variations of the latter pattern are used in various places in pair.c.

Another potential problem is that most sscanf calls have their return value checked against zero (if (!sscanf(...))), instead of checking for zero or negative values. As sscanf may also return EOF in case of matching failure, I'm not sure whether this is right.

I haven't figured out how to fix these issues yet, so no suggested patch this time. :(

Discussion

  • Albert Graef

    Albert Graef - 2015-05-31

    Ok, I realized now that a lone comma on the lhs of a rule is still needed even if there are no arguments, so my example should actually be written:

    /1 , : noteon( 0, 60, 127 );
    

    And this works ok, so no bug there.

    I'm still worried about lines starting with whitespace being treated as comments, even if they have no # symbol on them. I'd like to suggest the attached change which fixes this, so that only lines beginning with # (possibly after some leading whitespace) are considered as comment lines.

     
  • ssj71

    ssj71 - 2015-06-01

    I was aware of the idiosyncrasy when no OSC arguments exist. I'm not sure if I've indicated it correctly in the docs. It would probably be better for users if I allow this special case where the comma is not there.

    As for the whitespace, I realize now that this is indeed a bug, the intention was that if the '#' were indented for whatever reason it would still recognize it as a comment. Patch applied (through your bitbucket fork). Thanks.

    I'm closing this bug, but I'll think about the no arg case for a bit. This will ripple through nearly all parts of the parser since it allows a second form of the config line template.

     
    • Albert Graef

      Albert Graef - 2015-06-02

      I'm closing this bug, but I'll think about the no arg case for a bit. This will ripple through nearly all parts of the parser since it allows a second form of the config line template.

      It's not really a big issue, just might be worth pointing out in the docs.

       
  • ssj71

    ssj71 - 2015-06-01
    • status: open --> closed
     

Log in to post a comment.