ronnie sahlberg wrote:
1,
can you refactor your patch to not use long and %ld format strings
since this type will differ between platforms.
see README.developer for instructions on what types and format strings to use.
I've checked in a change to fix that.
2,
dont reference the fields inside tvb directly ( tvb->length , use
tvb_length(tvb) instead )
I've checked in a change to use tvb_reported_length(tvb) to get the
packet length, as that's the actual length of the packet
(tvb_length(tvb) is just the amount of packet data that was captured,
which might have been cut short by "slicing"/a "snapshot length").
3,
why do you pass a pointer to offset to all routines, it is more
common in ehtereal to pass
offset as a normal parameter and then return the new alue for offset.
maintenance of the dissector will be much easier if it follows the
style of the other dissectors.
I didn't address that one...
...but I looked at the BACnet spec, and the application-layer protocol
is ASN.1-based. Unfortunately, they use their own encoding rules - they
do their own special encoding, with no explicit tags, in the fixed
portion of the PDU, and use BER in the variable portion - so I don't
know whether the dissector can be generated with asn2eth (and the
license for the protocol spec is a bit obnoxious, so I don't even know
whether the ASHRAE's lawyers will come after you if you make public the
ASN.1 specification).
Even if we can't generate the dissector from the ASN.1 for the protocol,
using the packet-ber.c routines for the variable portion might be the
right thing to do.
