Michael Mann
changed
bug 9123
| What |
Removed |
Added |
| Attachment #11528 Flags |
review_for_checkin?
|
review_for_checkin-
|
Comment # 4
on bug 9123
from Michael Mann
Comment on attachment 11528 [details]
Patch for adding sync frames support and support for additional feature flags.
A few issues with the patch:
1. Please use proto_tree_add_item as much as possible, as it saves creating
"temporary" variables to use with proto_tree_add_uint/boolean. There is also a
FT_ETHER type so proto_tree_add_item can be used for "ether" addresses.
2. Consider using proto_tree_add_bitmask for your groups of booleans.
3. No need to have a NULL tree check for each proto_tree_add_xxx call. All
proto_tree_add_xxx calls have a NULL tree check within them, so it's really
only a (very small) performance savings to do a NULL tree check over a large
group of proto_tree_add_xxx calls.
Also please fuzztest with the capture provided.
You are receiving this mail because:
- You are watching all bug changes.
