On 12/12/2011 4:55 PM, Bill Meier wrote:
Summary
-------
I've recently been digging into the tvbuff code.
After doing so, I've come to the conclusion that the current tvbuff
implementation of managing tvb's with usage_counts and used_in lists:
- doesn't really provide some of what I understand to be
the intended functionality;
- is not necessary given the ways that Wireshark actually uses tvbuffs;
- can be significantly simplified by doing away with usage counts
and used_in lists.
[snip]
Proposal
--------
>
> [snip]
Ok:
I've committed new versions of tvbuff.h, tvbuff-int.h and tvbuff.c which
implement the proposal as described previously. (See below for one
difference).
The changes simplify the code and should minimize if not eliminate
tvb leakage (other than for tvbs created by dissectors via
tvb_new_real_data()).
I've done a fair amount of testing, but, obviously, "the proof of the
pudding is in the eating" so we'll see how things go.
ToDo:
- Add/update info about dissector tvb usage to doc/README.developer.
- Update epan/tvbtest.c
- Test composite tvbs (They might now even work).
- ???
-----------
From the commit log:
A simplified version of tvbuffs:
- Essentially no changes from current dissector de facto tvbuff usage;
- Do away with 'usage_counts' and with 'used_in' GSLists;
- Manage tvb chains via a simple doubly linked list.
- API changes:
a. tvb_increment_usage_count() and tvb_decrement_usage_count() no
longer exist;
b. tvb_free_chain() can only be called for the 'top-level' (initial)
tvb of a chain) or for a tvb not in a chain.
c. tvb_free() now just calls tvb_free_chain() [should have no impact
on existing dissectors].
--------
Note: The only difference from the original proposal is that tvb_free()
is still callable from a dissector; tvb_free() now just calls
tvb_free_chain().
This was done so that existing dissectors calling tvb_free() need not be
changed.
(I'd argue that dissectors actually should have been calling
tvb_free_chain() for tvbs created using tvb_new_real_data() since that
handles the case wherein any subsets, etc had been created on the tvb).
In any case, I believe having tvb_free() just calling tvb_free_chain()
should not impact dissectors which use tvb_free().
Bill
