On Mon, Oct 22, 2012 at 12:21 PM, Anders Broman
<anders.broman@xxxxxxxxxxxx> wrote:
>
>
> -----Original Message-----
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] 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
