Wireshark mailing list archives
Re: Simplifying (and fixing) tvbuff [Long]
From: Bill Meier <wmeier () newsguy com>
Date: Wed, 21 Dec 2011 12:40:58 -0500
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 eliminatetvb 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 ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Simplifying (and fixing) tvbuff [Long] Bill Meier (Dec 12)
- Re: Simplifying (and fixing) tvbuff [Long] Bill Meier (Dec 12)
- Re: Simplifying (and fixing) tvbuff [Long] Bill Meier (Dec 13)
- Re: Simplifying (and fixing) tvbuff [Long] Chris Maynard (Dec 14)
- Re: Simplifying (and fixing) tvbuff [Long] Jaap Keuter (Dec 14)
- Re: Simplifying (and fixing) tvbuff [Long] Chris Maynard (Dec 14)
- Re: Simplifying (and fixing) tvbuff [Long] Bill Meier (Dec 21)
- Re: Simplifying (and fixing) tvbuff [Long] Bill Meier (Dec 21)
- Re: Simplifying (and fixing) tvbuff [Long] Jaap Keuter (Dec 24)
- Re: Simplifying (and fixing) tvbuff [Long] Bill Meier (Dec 21)