-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 I've got some stylistic questions.
It looks like the SLP support in ethereal has accumulated a lot of bit-rot. I cannot test the SLPv1 support - don't know it currently works well, or not. However if I convert SLPv2 across to the newer routines, it'd generally be better to make the SLPv1 stuff the same. Except that I can't test, and might destablise the SLPv1 support. Is this OK with the current state of ethereal? Do people normally in-line the dissector, or is considered better to write smallish parser routines and call them as needed? More below: On Sun, 29 Sep 2002 14:10, you wrote: > Some comments: > Some value_strings have very long text fields. It might be difficult to see > them all on a small screen since they might run off the right edge of the > screen. Can they be made shorter/more compact? These were cut'n'pasted from the RFC, because I'm lazy. Now fixed. > Your patch, as well as the previous code, use proto_tree_add_text to print > teh booleans for a bitmask. The Overflow, monolingual etc ones. > These would be much better if they were broken out into a separate function > that only dissects the bitfields as booleans. > Please see say packet-smb.c for how FT_BOOLEANs are used. > This requires a TRUE_FALSE_STRING as well for every field but makes it > possible to search for these fields using display filters. Now OK for V2. I'll do V1 if people don't mind that it might break. > There are a lot of proto_tree_add_text() that just prints some text and a > value from tvb_get_ntoh?(). > It would be much better if these fields got their own hf_field of type > FT_UINT??, > then you could use proto_tree_add_item() instead and the fields could also > be used in display filters since then they have names. > Please consider replacing all the proto_tree_add_text() with appropriate > hf_ and proto_tree_add_item() instead. Hmmm. Lots of work. What advantage does this provide to the user? > At at least one place you extract a value in seconds in unix epoch and then > use gmtime() to break it up before you use proto_tree_add_text() to display > it. > A much nicer way is to use a FT_ABSOLUTE_TIME hf_field and use tha > appropriate function which will do all this for you. > Please see packet-smb.c or any other dissector using FT_ABSOLUTE_TIME for > how to use it. Still to look at this. Is there some "hackers guide to ethereal"? Or is just by example? Brad - -- http://conf.linux.org.au. 22-25Jan2003. Perth, Aust. Tickets booked. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.6 (GNU/Linux) Comment: For info see http://www.gnupg.org iD8DBQE9ltTiW6pHgIdAuOMRAl64AJ9DvpxjzYJNsJD3llFaQWXQFEr9AgCglspj h4dYM53Ch85RpKsd9eW+ZW8= =b944 -----END PGP SIGNATURE-----
