https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5095
--- Comment #8 from Bill Meier <wmeier@xxxxxxxxxxx> 2010-08-30 16:05:54 EDT ---
More comments:
1. We prefer that the proto_reg_handoff be at the end of the file
(after the proto_register fcn). (You will need a forward ref to
proto_reg_handoff).
2. proto_reg_handoff: the heuristic_dissector_add() should only be done once;
Ie: put it under the 'if(...initialized)'.
3. It appears that gbl_keytab_filename is really a directory (as indicated by
the text and code). So: For clarity I suggest using gbl_keytab_directory or
gbl_keytab_folder or something similar as the variable name.
Also: I'm guessing that using the plugin_dir as the default directory is
left
over from when this dissector was coded as a plugin.
(It sounds like this dir/folder isn't really a "configuration"
kind of thing an so wouldn't be stored in the Wireshark configuration dir).
I suggest maybe using "" as the default dir in the preferences. Then in
read_hashed_symbols_from_dir(), if the dir is other than "" you can give an
error message if the dir can't be opened.
See packet-ssl.c for the use of report_open_failure().
prefs_register_string_preference(etch_module, "file",
"Apache Etch symbol folder",
"Place the hash/symbol files (generated by the Apache Etch
compiler) ending with .txt here",
&gbl_keytab_filename);
4. #includes not req'd ?
io.h
addr_resolv.h
oids.h
tvbuff.h
filesystem.h (if get_plugin_dir not used)
5. typo: lenght == > length
6. The symbol values in the key table file are in hex.
Is it useful/necessary to show the 'etch symbol' in the dissection as an
int ??
7. I'm not sure I understand the code related to
/* Switch to another frame? => ... */
Can you provide a capture file which exercises this code so I
can dig a bit deeper ?
It looks like such a capture file might contain at least two different
cases:
a) An ETCH PDU which starts at the beginning of a TCP frame and
extends to the next frame.
b) More than one ETCH PDU in a tcp frame with a partial ETCH PDU at
the end of the frame.
Does this sound right ?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
