Wireshark-bugs: [Wireshark-bugs] [Bug 8741] add UDT protocol support

Date: Fri, 31 May 2013 16:00:26 +0000

Comment # 4 on bug 8741 from
(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.