Comment # 4
on bug 8741
from chas williams (CONTRACTOR)
(In reply to comment #2)
> 1. Header file isn't necessary. #defines can be rolled into the .c file
> unless you expect them to be shared by other dissectors
OK
> 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
OK
> 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)
If I do it this way, I don't get a [Data] tree for the payload section of the
UDT data packets. I believe I am doing the right thing but it doesn't work the
way I expect when I do it the right way. Yes, that section of the code is a
bit hard to read. I will rewrite it.
> 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).
OK
> 5. It looks like the ""Missing Sequence Numbers" would be better as expert
> info
The NAK can contains a single sequence number or a list of sequence numbers.
That seems to make it clumsy to represent.
> 6. Also need to update epan/CMakeLists.txt to include your dissector.
OK
> Also please provide a sample capture for fuzz/regression testing.
OK
You are receiving this mail because:
- You are watching all bug changes.
