[bugzilla-daemon@xxxxxxxxxxxxx]

Evan Huus changed bug 9612

What	Removed	Added
Status	UNCONFIRMED	INCOMPLETE
CC		eapache@gmail.com
Ever confirmed		1

Comment # 4 on bug 9612 from Evan Huus

Hi Pavel, thanks for the patch. It looks really good already, just a few minor
comments:

- The explicit THROW_ON in dissect_amqp_1_0_list seems to duplicate what the
AMQP_INCREMENT macro does in the next line?

- Some of the other explicit THROW_ON calls are unnecessary if the dissection
would naturally just run off the end of the packet (which Wireshark already
catches and handles automatically).

- You set a lot of items as type FT_NONE when they appear to have normal types
(FT_BOOL, FT_UINT, etc). It is usually preferred to add these to the tree with
their correct type and value so that they can be filtered on by more than
simply their presence.

Cheers,
Evan

---

      You are receiving this mail because:

• You are watching all bug changes.