Wireshark-bugs: [Wireshark-bugs] [Bug 8673] [PATCH] - GigE Vision GVSP dissector
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: bugzilla-daemon@xxxxxxxxxxxxx
Date: Wed, 15 May 2013 10:32:00 +0000

changed bug 8673

What Removed Added
CC   pascal.quantin@gmail.com

Comment # 3 on bug 8673 from 
Hi,

thanks a lot for your submission. You will find below a first feedback based on
a really quick review:
- we prefer to integrate built-in dissectors rather than plugins in the
official Wireshark. Please convert it.
- the hf_register_info structure should be put as static inside the
proto_register_gvsp function. See doc/packet-PROTOABBREV.c sample code for an
example
- you can remove the call to check_col() and always call the col_XXX()
functions now (check_col() is deprecated)
- the last argument for proto_tree_add_item is no more TRUE/FALSE. Instead it
should be ENC_BIG_ENDIAN, ENC_LITTLE_ENDIAN, ENC_NA, ENC_ASCII,... depending on
the filed type. See doc/README.developer for details
- the blurb parameter for each header field as no added value ("GVSP Pixel
Format" vs "Pixel Format" for example). Why not drop it and put NULL instead?

That's all I have in mind for now. The patch will get a deeper review by
someone once those first comments are addressed I think.

You are receiving this mail because:
  • You are watching all bug changes.