Hello, Vojtech, Vojtech Horky on 2012-08-01: > > I implemented "if" today, but documentation, examples, and "switch" will > > have to wait for tomorrow. > The functionality side is okay. I played with it a bit and it works fine. > > However, you are still behind your (revised) schedule. And the amount > of commits (code) does not indicate any increased effort. At least > that is the way it looks to me...
I have been having some trouble focusing while working from home, but I managed to work much more today. I'm designing the repetition support, which seems like it will require handling scopes differently. > During last days I also did a quick review of your new code and I have > a few comments. > > In general, the code looks clean and tidy. Also, you are consistent > and the code is well structured. > > Now, concretely: in parse_if, you jump into a conditional block. I > would consider that a very bad practice and actually I am surprised > that such jump does not produce any warning. Better is to go with the > common practice and put the error block at the end of the function. Well, the whole purpose of goto is to interfere with the control flow. It could be confusing, though, so I fixed it by moving some code to a separate function. > In the same function, it looks to me that there is a leak in case > bithenge_if_transform() fails. > > Another leak is in parse_expression() - if the > bithenge_const_expression() fails, you do not destroy the previously > created node. I have a general principle that functions like bithenge_if_transform() and bithenge_const_expression() should reliably free their arguments, even if an error occurs. I've added an explanation to the wiki page[0]. bithenge_if_transform() follows this principle and does not leak, even on error (I checked with Valgrind). However, I forgot to free the node in bithenge_const_expression() when an error occurs, so it was a leak. Thanks for pointing this out. Thanks, Sean [0] http://trac.helenos.org/wiki/StructuredBinaryData#UsingtheAPI _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
