Evan Huus
changed
bug 9072
| What |
Removed |
Added |
| Attachment #11566 Flags |
review_for_checkin?
|
review_for_checkin-
|
Comment # 32
on bug 9072
from Evan Huus
Comment on attachment 11566 [details]
enhancement packet-mq
This looks pretty good, I just have a few minor comments now:
- In packet-mq.c, emem.h is included but not used that I can see?
- At packet-mq.c:3192, use wmem_new instead of wmem_alloc to save yourself
writing the sizeof() unnecessarily.
- You mostly use tabs for indentation, which is fine, but there are a few
places where you mix tabs and spaces, which is confusing. Adding modelines [1]
to the bottom of the files will probably help. There are also many lines with
trailing whitespace which it would be nice to remove.
- When compiling, I get many errors like the following:
packet-mq.h:39:26: error: pasting "MQ_MQIA_TCP_KEEP_ALIVE" and "," does not
give a valid preprocessing token: #define DEF_VALS2(A,B) { MQ_##A##, B }
I believe that the second "##" is unnecessary in this macro and in DEF_VALS1?
- We have a script ./tools/checkhf.pl which prints the following warnings:
Unused entry: epan/dissectors/packet-mq.c, hf_mq_fcno_vercli
Unused entry: epan/dissectors/packet-mq.c, hf_mq_fcno_versvr
Unused entry: epan/dissectors/packet-mq.c, hf_mq_id_unknown05
Unused entry: epan/dissectors/packet-mq.c, hf_mq_id_unknown20
Unused entry: epan/dissectors/packet-mq.c, hf_mq_lpoo_option
Unused entry: epan/dissectors/packet-mq.c, hf_mq_od_objectstring
Unused entry: epan/dissectors/packet-mq.c, hf_mq_od_resobjectstr
Unused entry: epan/dissectors/packet-mq.c, hf_mq_od_selectstring
The Cppcheck tool ([2] and ./tools/cppcheck/cppcheck.sh) also prints several
warnings and errors.
[1] https://www.wireshark.org/tools/modelines.html
[2] http://cppcheck.sourceforge.net/
You are receiving this mail because:
- You are watching all bug changes.