Wireshark · Ethereal-dev: [ethereal-dev] Dissector exceptions

[Gilbert Ramirez <gram@xxxxxxxxxx>]

Here's a rough draft of how exceptions could be implemented for
Ethereal's dissectors. To test, apply the patch and modify
Makefile.am to include 'except.c' and 'tvbuff.c' in
ETHEREAL_COMMON_SOURCES.  The only dissector that uses
exceptions in this rough draft is packet-fddi.c. (it was the
simplest top-level dissector to modify). Attached
is a sample trace file of short FDDI packets that I 
created with 'randpkt'.

Overview
--------

The exceptions module from the kazlib library was added.
This is except.[ch]
'exceptions.h' is a set of macros to implement TRY, CATCH,
CATCH_ALL, FINALLY, ENDTRY, and THROW. This file also defines the
exceptions for Ethereal, of which there is only one: BoundsError.

'tvbuff.c' implements a buffer class that raises exceptions
when attempts to read beyond its boundaries is made.
Three types of tvbuff's exist:

/* TVBUFF_REAL_DATA contains a guint8* that points to real data.
 * The data is allocated and contiguous.
 *
 * TVBUFF_SUBSET has a backing tvbuff. The TVBUFF_SUBSET is a "window"
 * through which the program sees only a portion of the backing tvbuff.
 *
 * TVBUFF_COMPOSITE combines multiple tvbuffs sequentually to produce
 * a larger byte array.
 *
 * tvbuff's of any type can be used as the backing-tvbuff of a
 * TVBUFF_SUBSET or as the member of a TVBUFF_COMPOSITE.
 * TVBUFF_COMPOSITEs can have member-tvbuffs of different types.
 *
 * Once a tvbuff is create/initialized/finalized, the tvbuff is read-only.
 * That is, it cannot point to any other data. A new tvbuff must be created if
 * you want a tvbuff that points to other data.
 */

Right now, only TVBUFF_REAL_DATA and TVBUFF_SUBSET are implemented.

dissect_packet() creates a tvbuff and passes it to dissect_fddi().
dissect_fddi() uses the tvb_get*() routines to retrieve data from
the packet. The FDDI dissector then creates a new tvbuff of type
TVBUFF_SUBSET. It is a tvbuff which contains the payload of the
FDDI packet (i.e., w/o the FDDI header). dissect_fddi() would then
pass this new tvbuff to the next dissector, except for the fact that
no other dissector currently uses tvbuffs.

tvb_compat() is used to retrieve values for 'pd' and 'offset' from the
tvbuff, to be passed to the next dissector. In this way we can
gradually move the dissectors over to the new format.

Remember how I wanted to move per-packet info that does not belong in
frame_data into a new struct? I realized that our packet_info struct would
serve just fine. I've moved 'fd' into packet_info, and have added a new
string, 'current_proto', to packet_info. Eventually I'll move 'cinfo' into
packet_info as well.

'current_proto' is set to the name of the protocol currently being dissected.
If an access to a tvbuff causes an exception, the TRY/CATCH block in
dissect_packet() catches the BoundsError (if the sub-dissector didn't, which
would most likely be the case) and adds this text to the protocol tree:

	"Short Frame: [%s]", pi.current_proto

So, the generic template for an tvbuff-aware dissector is:

	dissect(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)

('fd' is now in pinfo, and the functionality of 'pd' and 'offset' is now
handled by the tvbuff)

An interesting thing results from combining 'pd' and 'offset' in the tvbuff.
The offsets that we pass to the proto_tree_add_*() routines no longer make sense,
because the dissector no longer needs to know what the beginning offset of the
data is... the dissector always accesses data from offset 0, and the tvbuff
magically finds the right offset inside the packet. I get around this in
packet_fddi() only by avoiding the issue entirely... since the fddi header
is at offset 0, I can ignore it for this rought draft. I wanted to get something
running to present this conundrum for discussion.

One solution is to teach the proto_tree_add_*() routines about tvbuffs, and
probably provide both tvbuff-aware and tvbuff-non-aware versions of the
functions. Skeleteal code for packet-fddi would then look like:

  if (tree) {
        ti = proto_tree_tvb_add_protocol_format(tree, proto_fddi, tvb, 0, FDDI_HEADER_SIZE,
                "Fiber Distributed Data Interface, %s", fc_str);

      swap_mac_addr(dst_swapped, (u_char*) tvb_get_ptr(tvb, FDDI_P_DHOST, 6));
      swap_mac_addr(src_swapped, (u_char*) tvb_get_ptr(tvb, FDDI_P_SHOST, 6));

      fh_tree = proto_item_add_subtree(ti, ett_fddi);
      proto_tree_tvb_add_item(fh_tree, hf_fddi_fc, tvb, FDDI_P_FC, 1, fc);
      proto_tree_tvb_add_item(fh_tree, hf_fddi_dst, tvb, FDDI_P_DHOST, 6, dst);

... etc.

See how each call to proto_tree_add_* needs both the tvbuff pointer, which provides
the offset relative to the start of the frame, and the offset, which will provide
the offset relative to the start of the tvbuff?

I'm happy about how well the exceptions work, at least with short FDDI packets. I'm
a little upset about the ugliness of the resulting proto_tree_add*() calls.

Maybe some of Guy's routines to have the proto* routines extract data from the tvbuff
(in lieu of the dissector doing it) would help; perhaps the length would not have to be
passed to the proto* routine.

Another solution would to have the dissector extract the "offset from the beginning of
the frame" from the tvbuff before adding to the proto_tree. Then, like now, we'd have
calls that add to 'offset':

      offset = tvb_get_absolute_offset(tvb);
      proto_tree_add_item(fh_tree, hf_fddi_dst, offset+FDDI_P_DHOST, 6, dst);

Ideas?

--gilbert

Attachment: short-fddi.cap
Description: Binary data

Attachment: except.diff.gz
Description: Binary data