Wireshark-bugs: [Wireshark-bugs] [Bug 8633] New Dissector for Common Address Redundancy Protocol
Martin Kaiser
changed
bug 8633
What |
Removed |
Added |
Status |
UNCONFIRMED
|
IN_PROGRESS
|
CC |
|
wireshark@kaiser.cx
|
Ever confirmed |
|
1
|
Comment # 2
on bug 8633
from Martin Kaiser
Hi Uli,
thanks for the submission. This looks pretty good already.
Some (minor) review comments:
- please don't use a mixture of TABs and spaces for indenting
- please add editor modelines at the bottom of the file, see e.g.
packet-dvbci.c
- there's no need for check_col() before col_add_fstr()
- you don't need the huge if (tree) statement, the functions you call inside
can
all handle tree==NULL
- please make the dissector a new-style dissector that returns the number of
bytes it processed (you're counting them already)
Yes, I know that most of these issues exist (and should be cleaned up) in
packet-vvrp.c as well.
The main thing to clarify is how to deal with the conflict between vrrp and
carp.
One approach would be to make both vrrp and carp new-style dissectors who
reject the packet if it's not their protocol. Would it be possible to add
checks that detect a carp packet reliably (e.g. using version, type, min/max
length)?
Best regards,
Martin
You are receiving this mail because:
- You are watching all bug changes.