On Thu, 19 Jul 2007 14:16:49 -0400, Jeff Morriss wrote
> Didier wrote:
> > Hi,
> >
> > 1) It seems that since some glib 2.0 version g_mem_chunk_destroy doesn't
> > free
>
> The docs certainly seem to indicate that the memory should actually
> be freed:
>
> http://www.gtk.org/api/2.6/glib/glib-Memory-Chunks.html#g-mem-chunk-destroy
>
> and my (basic) attempt to follow the code goes down to
> g_slice_free1() which appears to at least _try_ to free the memory.
> Am I missing something?
In my understanding now g-mem-chunk-destroy only free the mem chunk object
(aka sizeof structure g_mem_chunk) not the allocated memory as with gtk1.
You have to call g_mem_chunk_free for each allocated chunk.
>
> [That's not to say I don't see memory usage growing when I reload a
> capture file, but I'm not convinced this is the source.]
Attached a small patch which replace gmemchunk with se_alloc (I didn't
change cf name and header file on purpose but a proper patch should).
The drawback is that if DEBUG_USE_CANARIES is defined in epan/emem.c (the
default) the virtual size is a lot bigger. There's still leaks though.
>
> > 2)COPY_ADDRESS is still misused in a lot of place it g_malloc address
> > space
> > but many don't free it.
>
> There is (also) an SE_COPY_ADDRESS; is there any reason not to make
> all COPY_ADDRESS calls seasonal?
It makes sense for some but a lot of them are in taps code with a live time
between ep and se.
Some leaks are on every access cf. dissectors/packet-jxta.c
Didier
=== modified file 'file.c'
--- file.c 2007-07-19 22:04:10 +0000
+++ file.c 2007-07-19 23:50:31 +0000
@@ -75,6 +75,7 @@
#include <epan/dissectors/packet-data.h>
#include <epan/dissectors/packet-ber.h>
#include <epan/timestamp.h>
+#include <epan/emem.h>
#include "file_util.h"
@@ -256,11 +257,7 @@
nstime_set_unset(&first_ts);
nstime_set_unset(&prev_dis_ts);
- cf->plist_chunk = g_mem_chunk_new("frame_data_chunk",
- sizeof(frame_data),
- FRAME_DATA_CHUNK_SIZE * sizeof(frame_data),
- G_ALLOC_AND_FREE);
- g_assert(cf->plist_chunk);
+ cf->plist_chunk = NULL;
/* change the time formats now, as we might have a new precision */
cf_change_time_formats(cf);
@@ -308,10 +305,8 @@
/* ...which means we have nothing to save. */
cf->user_saved = FALSE;
- if (cf->plist_chunk != NULL) {
- g_mem_chunk_destroy(cf->plist_chunk);
- cf->plist_chunk = NULL;
- }
+ cf->plist_chunk = NULL;
+
if (cf->rfcode != NULL) {
dfilter_free(cf->rfcode);
cf->rfcode = NULL;
@@ -1063,8 +1058,11 @@
int row = -1;
/* Allocate the next list entry, and add it to the list. */
- fdata = g_mem_chunk_alloc(cf->plist_chunk);
-
+ if (cf->plist_chunk)
+ fdata = cf->plist_chunk;
+ else
+ cf->plist_chunk = fdata = se_alloc(sizeof (frame_data));
+
fdata->num = 0;
fdata->next = NULL;
fdata->prev = NULL;
@@ -1108,16 +1106,7 @@
cf->f_datalen = offset + phdr->caplen;
fdata->num = cf->count;
row = add_packet_to_packet_list(fdata, cf, dfcode, pseudo_header, buf, TRUE);
- } else {
- /* XXX - if we didn't have read filters, or if we could avoid
- allocating the "frame_data" structure until we knew whether
- the frame passed the read filter, we could use a G_ALLOC_ONLY
- memory chunk...
-
- ...but, at least in one test I did, where I just made the chunk
- a G_ALLOC_ONLY chunk and read in a huge capture file, it didn't
- seem to save a noticeable amount of time or space. */
- g_mem_chunk_free(cf->plist_chunk, fdata);
+ cf->plist_chunk = NULL;
}
return row;
