Comment # 4
on bug 9033
from Jakub Zawadzki
(In reply to comment #3)
> (In reply to comment #2)
> > > There are probably two issues here:
> > > - rescan_packets should bail because there is no file open
> > > - if it doesn't bail, it should clean up the epan it creates so that wmem
> > > leaves file scope
> >
> > - wmem shouldn't do stupid assertions.
>
> These are the kind of weird bugs that those assertions are meant to catch.
> If it hadn't asserted there is the possibility of all sorts of bizarre
> subtle errors.
What about removing wmem_epan_scope(), wmem_file_scope(), wmem_packet_scope()?
Replace with helper functions:
proto_alloc() <-- alloc memory for libwireshark
epan_alloc(epan) <-- alloc memory for given session
packet_alloc(pinfo) <-- alloc memory for given packet
(These two I think are mostly used, so it'd be great if we have some helpers,
but we don't strictly need to)
In other cases pass pool to some function:
wmem_foo(epan_pool(epan), ...)
wmem_foo(packet_pool(pinfo), ...)
If you have valid epan or pinfo pointer it's much more likely that it'll work
;]
I'm aware of trees created in proto_register_ routines,
but if we support multi-files, we'd anyway need some functions for dissectors
'call me when new session is created, destroyed' (best auto generated - like
proto_reg_*, proto_register_*).
> The broader problem is that we unconditionally call epan_free (previously
> cleanup_dissection) in places like cf_open (tshark.c:3806) where we don't
> necessarily have a file open.
> epan_free should be called when the old file
> is closed, not when the new file is opened.
Why? free(0) is NOP. It'd be best if epan_free(0) would be also NOP.
> Rescan_packets has an 'excuse'
> in that it can reasonably expect to only be called if there is a file open.
If we really need to.
btw. what about testing for cf->epan? It's shorter than cf->state !=
FILE_CLOSED.
You are receiving this mail because:
- You are watching all bug changes.