Ethereal-dev: RE: [Ethereal-dev] support for X11 replies and events

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Biot Olivier <Olivier.Biot@xxxxxxxxxxx>
Date: Thu, 25 Dec 2003 01:54:32 +0100
| From: Michael Shuldman

[...]

| When doing a quick test now I noticed that it crashed due
| to the following code in dissect_x11_request():
| 
| /* In the interest of speed, if "tree" is NULL, avoid building a
|    protocol tree and adding stuff to it if possible.  Note,
|    however, that you must call subdissectors regardless of whether
|    "tree" is NULL or not. */
|         if (!tree)
| 		return;

If tree is NULL, then ethereal/tethereal is not interested in the *details*
of the protocol. The if (tree) construct allows a developer to short-circuit
code that only displays protocol details (proto_XXX_DO_YYY() calls). It is
important however that you don't put important protocol state computations,
packet reassembly, conversation setup etc into such a block, otherwise you
need a dfilter or you need to select a given packet in order to have this
computed (since then tree will be different from NULL).

To word it differently:

1. if tree is NULL,
then [t]ethereal should only run the non-display code that is required to
correctly understand the protocol (check the validity if applicable, update
the Info and Protocol column, build conversations if applicable, same for
reassembly if the protocol supports this, maybe state information like
matching requests with replies or keeping track of session capabilities, and
of course also the handoff to other dissectors.

2. if tree is not NULL,
then [t]ethereal wants to render the protocol in all its glory (read: be
able to access all internal details of the protocol such as the different
protocol field names and their values, etc. as required by the evaluation of
a dfilter).

| That didn't work since we need run through some of the code
| coming in order to handle the later reply.  I don't know 
| why we get a NULL tree now but not in 0.9.7.  I don't remember what
| a NULL tree means either, so I removed the return statement.

This was an outstanding bug where a tap always started one or more display
filters (dfilters), so the entire protocol tree was evaluated hence tree was
set to a value different from NULL.

| Also one other problem, tcp desegmentation needs to be switched
| on for the parsing of keypresses and related stuff to work,
| the X11 keyboardmap is too big to fit in a single ip packet.

It is not a problem, the TCP protocol provides a protocol preference
allowing the end-user to control TCP reassembly for higher-level protocols.
This gives me an idea: maybe we should provide a small info field for
protocol entries. This information could then also be output with the -G
option.

| I want to thank Christophe Tronche <ch.tronche@xxxxxxxxxxxx> (who
| wrote the orignal X11 code here) for the tips he gave me before I
| had yet started on the work, that and his code made the work quite
| a bit easier.
| 
| I also want to thank Open-IT (www.open-it.com) for letting me
| give the code back to your project.

Regards,

Olivier