----- Original Message -----
From: "Brad Hards"
Sent: Sunday, September 29, 2002 1:39 PM
Subject: Re: [Ethereal-users] SLPv2 support
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Sat, 28 Sep 2002 06:47, Ronnie Sahlberg wrote:
> > Give it a try and post your patch to ethereal-dev,
> > the people there can provide comments on how to
> > improve it/clean it up.
> First cut patch attached. I am a newbie to ethereal (even as a user), so I
> don't know if there are some tragic style or coding errors in this.
>
> Feedback appreciated - please be gentle, this is my first time....
>
> Brad
>
> BTW: Is there some set of test files (eg in libpcap form) that I can
> contribute my tests to?
Looks really good for a first try.
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?
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.
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.
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.
best regards
ronnie sahlberg
