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.
On 2010/03/12 22:25:36, zundel wrote:
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?
Clarified.
http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode40
Line 40: #define DEFAULT_GWTCS_PORT 9997
On 2010/03/12 22:25:36, zundel wrote:
this declaration could be externalized - 9997 is also declared as a
constant in
../common/HostChannel.h
Not without refactoring the existing code, since the dissector has to be
pure C and HostChannel.h is C++.
http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode279
Line 279: gint32 i,j;
On 2010/03/12 22:25:36, zundel wrote:
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.)
Rewrote significantly as part of work to improve packet decoding.
http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode366
Line 366: j += 4 + i;
On 2010/03/12 22:25:36, zundel wrote:
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.
I really hate macros with side effects (since it obscures the update),
and without doing that I didn't see anything that would be better than
what is there now.
http://gwt-code-reviews.appspot.com/191801/diff/1/4#newcode389
Line 389: #if 0
On 2010/03/12 22:25:36, zundel wrote:
still need this code? Maybe just leave a comment instead.
Was there as part of the skeleton for decoding arguments, so I finished
that.
http://gwt-code-reviews.appspot.com/191801
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
