https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6373
Bill Meier <wmeier@xxxxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |NEW
--- Comment #8 from Bill Meier <wmeier@xxxxxxxxxxx> 2011-09-27 14:03:31 EDT ---
Additional notes: :)
1. There are a number of unreferenced value_string arrays:
main_cap, playback_fmt_vals, warn_codes, info_codes, ...
2. The preferred source file format: proto_register...() followed by
proto_reg_handoff...() at the end of the file.
3. TCP ports 5911 & 5912 do not appear to be assigned to SPICE in the IANA port
list but appear to be assigned for something else. Are there IANA assigned
ports for SPICE ?
See:
http://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xml
If there are no assigned ports for SPICE, then just use the heuristic and
possibly the "alternate port" preference.
4. The "alternate port" preference is commented out but the related code and
variables are not. Do you want the alternate port preference capability ?
If yes, please see the packet-bvlc.c dissector for an example of how to do this
(which is slightly different from that in packet-spice.c).
5. spice.magic is shown as FT_STRING so ENC_NA is the correct encoding in the
proto_tree_add_item() call.
6. Given that the magic field is a string, please use tvb_memeql() when testing
the field against "REDQ".
7. It appears that all the usage of val_to_str() has a constant string as the
3rd arg. Using val_to_str_const() instead in these cases is an optimization
which will save some memory allocation behind the scenes.
8. proto_reg_handoff(): The 'find_dissector("image-jfif")' should be under the
'if(!initialized)' since it needs to be called only once.
9. $Id$ is missing from the file.
(See doc/README.developer); (Search for $Id$).
10. 'proto_tree_add_item(tree, proto_spice, ...)' should use ENC_NA in all
cases.
11. There are several uses of tvb_length...() which I think should be
tvb_reported_length...(). Did you use tvb_length...() in these specific cases
for a reason ?
12. Almost all the ints displayed using proto_tree_add_item() are
ENC_LITTLE_ENDIAN.
A few are ENC_BIG_ENDIAN. Is this correct ?
Specifically: hf_session_id, hf_LZ_major_version, hf_LZ_minor_version
13. #include <stdio.h> & #include <string.h> aren't needed.
#include <.../packet-tcp.h> isn't needed ?
14. checkhf.pl gives
perl ...\checkhf.pl packet-spice.c
unused entry: packet-spice.c, hf_cursor_data_size
unused entry: packet-spice.c, hf_Clip_data
15. The attached capture file, when opened (on Windows) in Wireshark (with or
without the SPICE dissector) shows many frames which show as VNC but which are
not dissected further (even for VNC).
Is there something special which must be done to enable SPICE dissection ?
16. Whitespace:
- Some lines have trailing whitespace; it would be appreciated is this is
removed.
- What is the intended indentation width: 4 or 8 ?
If 4, then please use spaces iso "4 space tabs".
17. Has the dissector been fuzz-tested ?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
