LGTM, just some food for thought.
http://gwt-code-reviews.appspot.com/191801/diff/1/4 File plugins/wireshark/packet-gwtcs.c (right): http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode20 Line 20: * things like declaring all variables at the beginning of a block. Do you mean to avoid declaring all variables at the beginning of the block or to always declare all variables at the beginning of the block? http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode40 Line 40: #define DEFAULT_GWTCS_PORT 9997 this declaration could be externalized - 9997 is also declared as a constant in ../common/HostChannel.h http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode279 Line 279: gint32 i,j; The use of i,j below is a bit confusing to me. To me it would be clearer to create a third variable to use to walk through the packet (e.g. gint32 offset;) or to create a separate block {} for each case and give the variables meaningful names. (personal style: I usually reserve i,j,k for iterating and would use a different name for storing temp values read from the packet.) http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode366 Line 366: j += 4 + i; I don't see any bugs, but it seems like you could come up with a little pattern or macro here to make sure the size you increment j by always matches the size passed to proto_tree_add_item. http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode389 Line 389: #if 0 still need this code? Maybe just leave a comment instead. http://gwt-code-reviews.appspot.com/191801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
