https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6156
--- Comment #8 from Bill Meier <wmeier@xxxxxxxxxxx> 2011-08-04 15:14:04 EDT ---
Addt'l comments:
1. In general, tvb_reported_length() should be used instead of
tvb_length().
tvb_length() reports the actual amount data captured (which may
be less than the real size of the frame if s 'snapshot length'
was used during the capture.
tvb_reported_length() gives the real length of the frame (even if
only the first part was captured/saved).
That being said:
In general, a much better style iso constantly checking
tvb_reported_length() is to just proceed with the dissection
based upon the length fields in the frame, adjusting 'offset'
as you go. If something goes wrong (e.g., less bytes available
in a field than the length field specifies), the dissector will throw
an exception when trying to access the field and a "malformed"
indication will be shown for the frame.
For TCP (a "streaming" protocol), depending upon the
frame length to know how to proceed is normally
(almost always ?) not the right idea.
(A quick look suggests that your code depends
upon all the data for a message being present in one frame.
This might be the case most of the time, but is not guaranteed.
However, let's leave that for later).
2. Compile warnings on Windows using VC2008
packet-hdfs.c(102) : warning C4018: '<=' : signed/unsigned mismatch
packet-hdfs.c(119) : warning C4018: '<=' : signed/unsigned mismatch
packet-hdfs.c(133) : warning C4018: '<=' : signed/unsigned mismatch
(These probably get addressed after reworking the code as described
in item #1 above).
3.Please strip trailing whitespace from lines.
4. It appears that TCP port 8020 is assigned by IANA as follows:
intu-ec-svcdisc 8020/tcp Intuit Entitlement Service and Discovery
intu-ec-svcdisc 8020/udp Intuit Entitlement Service and Discovery
intu-ec-client 8021/tcp Intuit Entitlement Client
intu-ec-client 8021/udp Intuit Entitlement Client
http://www.iana.org/assignments/port-numbers
So: I suggest adding a preference to specify the TCP port to be used.
If there's no assigned port (which I have the impression is the case)
then the dissector_add_uint() should be called only if the
TCP port preference is set to non-zero.
See epan/dissectors/packet-fcgi.c for an example.
5. IMHO 'if (!success)' to test for "status == success" is kind of
misleading (but perfectly valid C) to someone casually reading the code.
Maybe: 'if (success == 0)' or even
'if (status == 0)' ??
6. Please use tvb_memeql() iso tvb_get_ptr()/strncmp() & etc
Let's start with the above and then we'll see if there are further comments.
Writing Wireshark dissectors takes a bit of work to learn the style and
the API but we'll get there.... :)
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
