Wireshark-bugs: [Wireshark-bugs] [Bug 8334] [BACnet] NPDU DLEN of 7 is shown to be invalid

Date Prev · Date Next · Thread Prev · Thread Next
Date Index · Thread Index · Other Months · All Mailing Lists
From: bugzilla-daemon@xxxxxxxxxxxxx
Date: Sat, 24 Aug 2013 11:38:42 +0000

changed bug 8334

What Removed Added
Attachment #11440 Flags review_for_checkin? review_for_checkin-

Comment # 9 on bug 8334 from 
Comment on attachment 11440 [details]
Patch to correct DLEN > 7 display

Hi Lori, thanks for the patch. It has a few small issues that should be fixed
before I commit it:

- The idea to make a single 'addr' string field is a good one, however this
breaks typed filtering (since Wireshark now only know about the field as a
string). Is it possible to add both the typed and the 'general' field for each
case? The string field should probably be marked as generated
(PROTO_ITEM_SET_GENERATED) to distinguish them.

- ep_alloc(n) is deprecated, you should just be able to call
wmem_alloc(wmem_packet_scope(), n) instead.

- The fact that you hard-code the buffer size as 30 in multiple functions is
pretty fragile to future changes. Could you please use a #define of that value
somewhere so it only lives in one place?

- fByteToInt and fUnsigned32 seem to be doing almost exactly the same thing
(except one of them handles only lengths up to 4). Is there a reason to have
both?

Thanks,
Evan

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