-----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-----
