On Mon, Oct 22, 2012 at 12:21 PM, Anders Broman <[email protected]> wrote: > > > -----Original Message----- > From: [email protected] > [mailto:[email protected]] 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 <[email protected]> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
