Wireshark · Ethereal-dev: Re: [ethereal-dev] Re: Wishlist #32 (and 30) (oh and some possible UI enhancements)

[Guy Harris <gharris@xxxxxxxxxxxx>]

> The question I have to ask (and hopefully someone can answer me) is,
> why is the row number being stored in the frame_data->row field?  Is
> this just used for sequencing the packets as the come in?

It's used to indicate which row in the display - if any (remember, there
might be a display filter in effect, so some frames might not be
displayed at all) - the frame in question occupies.

> If you are
> using this value as the sequence number, then what is the use of the
> packet "No." column data?

The "column_info" structure contains data that is filled in when a frame
is dissected; the column data is then used to construct the GtkCList
entry for the packet.

It is *NOT* stored permanently - the "cinfo" member of a "frame_data"
structure is set to point to the "cinfo" member of the "capture_file"
structure, so when a frame is dissected, the column data for the
previous frame is trashed.

> Shouldn't we be using that instead of
> storing it redundantly in the frame_data->row field?

No, because it's not stored permanently.  You'd have to yank it out of
the GtkCList entry for the frame - unfortunately, to get that, I think
you'd have to know the row number....

> The solution in my mind would be to
> 	- when selecting the current row,
>         retrieve the packet number from the "No." column
> 
> 	- take this packet number and
>         pass it as an argument to the select_packet
>         (instead of the clist row)

*If* we can do so without turning the CList-construction behavior from
linear to quadratic (which I suspect we can do, with our patched version
of a GtkCList), I think we could, alternatively, use
"gtk_clist_set_row_data()" to associate with each row in the CList a
pointer to the "frame_data" structure for the row, and have
"select_packet()" use that to find the row, rather than scanning the
list of frames for the frame with the specified row number.

(I think we did that once upon a time, and I got rid of it because it
made the CList-construction quadratic, which meant that reading in a
Really Big Capture File wasn't just slow, it was *really really* slow;
later, some other stuff wasn't quite so easy to eliminate, so I fixed
the GtkCList code so that doing stuff to the *last* row could be done in
constant time rather than in a time proportional to the size of the
list, which I think makes doing a "gtk_clist_set_row_data()" right after
adding an entry to the list no longer make list-construction quadratic
in the ultimate size of the list.)

That also handles the case where we have a frame and need to know what
row in the GtkCList corresponds to it - use
"gtk_clist_find_row_from_data()" (as long as we don't use it very often
- we don't want to have to call it once for every single frame, say, as
that gets quadratic...).

> So how about we make all the columns of the packet_list available at
> startup?  All that the column preferences would do would be to say
> what order the columns appear and what order they appear in.  Any
> columns that we are not interested in would be hidden.
> 
> This design change would:
> 	- allow us to display/hide/move columns dynamically

Well, display and hide, yes.

Move is a bit trickier - you have "gtk_set_column_visibility()", but you
don't have anything to move columns.  ("gtk_clist_set_reorderable()" is
documented in

http://developer.gnome.org/doc/API/gtk/gtkclist.html#GTK-CLIST-SET-REORDERABLE

as specifiing "whether the CList's columns are re-orderable"; however,
what it actually appears to do is control whether the CList's *rows* are
reorderable by dragging and dropping.)

> 	- take up more memory for the packet list
> 	  (you HAVE 1GB of ram, don't you?)

Umm, no, actually, I don't.  I have only 128MB.

Yes, this is an issue.  I'd really like to be able to handle Really Big
Capture Files without paging like mad.  Perhaps the added memory won't
be enough to cause a problem, but....

Long term, I *would* like to replace the GtkClist with something better.

The *main* thing I want is to have a widget that stores, with each row,
precisely one piece of data: a pointer.

The widget will have, associated with it, a callback function.  When a
row is to be displayed on the screen, the callback function will be
called, with, as an argument, that pointer; it should return, in some
fashion, the data to be put in the columns for that row.

When a capture is read in, the packet will be "dissected" to the extent
that

	1) any per-packet persistent data necessary to dissect the
	   packet in full will be associated with the packet (see
	   various messages from Richard Sharpe, and followups thereto,
	   for information on storing per-packet data);

	2) any data necessary to dissect *subsequent* packets will be
	   set up.

No protocol tree will be constructed (unless a read filter is in effect)
- and we won't even construct column data.

A row will be added to the CList-replacement, with the data being a
pointer to the "frame_data" structure for the frame.

I suspect this will considerably speed up the process of reading in a
capture.  Only if a frame is actually being displayed, or printed, or
filtered, will we bother doing any dissection work (yes, just filling in
the columns is expensive).

This would also speed up display changes such as changing the time
format - changing the time format would just involve changing an
indication of the format to be used, and causing the rows currently on
the screen to be displayed (by poking the CList-replacement widget
telling it to do so).  (Yes, in a very large file, changing the time
format can be expensive.)

We could also make it support moving columns, etc.; I think there are
other things we could improve as well (I could try to dredge up a list
of problems we could fix).