Wireshark mailing list archives
Re: RFD: Limiting scope of ep_ memory
From: Evan Huus <eapache () gmail com>
Date: Mon, 22 Oct 2012 12:43:49 -0400
On Mon, Oct 22, 2012 at 12:21 PM, Anders Broman <anders.broman () ericsson com> wrote:
-----Original Message----- From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] On Behalf Of Jakub Zawadzki Sent: den 22 oktober 2012 09:11 To: Developer support list for Wireshark Subject: Re: [Wireshark-dev] RFD: Limiting scope of ep_ memory On Tue, Oct 16, 2012 at 03:18:32PM +0000, Anders Broman wrote:I think it sounds like the right thing to do and as no one have any objections I think you might as well go ahead and check it in :-)Bug #7892. Some solutions to fix it: 1/ revert r45673 2/ call epan_dissect_fill_in_columns() before ep_free_all() epan_dissect_fill_in_columns() in 8 cases of 9 is called after epan_dissect_run(). Only complicated case is tshark, which pass edt pointer to print_packet(), which later not always call epan_dissect_fill_in_columns(). Still it won't work, if GUI use ep_ allocated addresses. 3/ don't use ep_ memory for pinfo-> addresses? Greping for e.g. \<net_src\> in GTK+ code shows that it's used also to build conversation filters (ip, ethernet, ...) for these addr->data points to tvb (tvbs are freed in epan_dissect_cleanup()). I haven't (yet) found use of address with AT_STRINGZ data, but it can change anytime.I'm no expert at this but some thoughts: - What is the ep scope e.g lifetime of it. I think the way you defined it looks good so that implies we should fix the problems that pops up. Does any one have other thoughts? If ep lifetime is the problem should we revert back and look at the packet list instead? Some of the optimizations calling for redissection is perhaps overkill or would it help to dissect a few more packets than the visible ones when scrolling. - Isn't all these problems latent in the code and could crop up at any time someting changes?
It's code that has been written to rely on the fact that ep memory didn't used to be freed until quite late. It's not necessarily *wrong* (although one could argue that the old ep behaviour was wrong) but it certainly won't work with the way ep memory is now being freed.
- I think the whole ep/se memory idea is optimizing for execution speed and as a bonus fewer memory leaks Does this assumption still hold true e.g. ep_ is faster that g_malloc? (Changing back would be a nightmare I suppose). But it seems like having ep memory is geting quite complicated.
I think reducing memory leaks is the primary goal, and execution speed was a bonus. I agree that it's getting overly complicated though, see my email from last week [1] for some thoughts on where I'd like to go long-term.
- My feeling is that mixing ep and g_malloced memory in pinfo is a bad idea.
Yes. Perhaps pinfo should have its own scope that is between ep and se? This would be much easier given the changes suggested in [1]. Cheers, Evan [1] https://www.wireshark.org/lists/wireshark-dev/201210/msg00178.html ___________________________________________________________________________ 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:
- RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 11)
- Re: RFD: Limiting scope of ep_ memory Evan Huus (Oct 11)
- Re: RFD: Limiting scope of ep_ memory Anders Broman (Oct 12)
- Re: RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 14)
- Re: RFD: Limiting scope of ep_ memory Martin Mathieson (Oct 14)
- Re: RFD: Limiting scope of ep_ memory Anders Broman (Oct 16)
- Re: RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Anders Broman (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Evan Huus (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Evan Huus (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Evan Huus (Oct 23)
- Re: RFD: Limiting scope of ep_ memory Anders Broman (Oct 12)
- Re: RFD: Limiting scope of ep_ memory Jeff Morriss (Oct 25)
- Re: RFD: Limiting scope of ep_ memory Evan Huus (Oct 11)
- Re: RFD: Limiting scope of ep_ memory Evan Huus (Oct 22)
- Re: RFD: Limiting scope of ep_ memory Jakub Zawadzki (Oct 22)