Michael Mann
changed
bug 8741
| What |
Removed |
Added |
| Attachment #10878 Flags |
review_for_checkin?
|
review_for_checkin-
|
Comment # 2
on bug 8741
from Michael Mann
Comment on attachment 10878 [details]
UDT protocol support diff against SVN head
Good start, here are some comments:
1. Header file isn't necessary. #defines can be rolled into the .c file unless
you expect them to be shared by other dissectors
2. The last parameter for proto_tree_add_item should be an ENC_* value, not
TRUE/FALSE. You can use fix-encoding-args.pl to help convert your existing
code
3. Why is the data dissector called in the "middle" of the dissector, with the
switch statement apparently "sharing" the same data. Are the "types" of the
switch statement only valid when "is control" is true? If that's the case, you
can just return the number of bytes used up to that point and the "dissection
architecture" will end up calling the data dissector for you (or perhaps
another dissector for that payload)
4. tvb_memcpy with proto_tree_add_bytes is unnecessary. You can just use
proto_tree_add_item (ENC_NA would be last argument value).
5. It looks like the ""Missing Sequence Numbers" would be better as expert info
6. Also need to update epan/CMakeLists.txt to include your dissector.
Also please provide a sample capture for fuzz/regression testing.
You are receiving this mail because:
- You are watching all bug changes.
