Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 45445: /trunk/epan/ /trunk/epan/: emem.c

[Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>]

On Wed, Oct 10, 2012 at 10:10:08AM -0400, Evan Huus wrote:
> > I still think that https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5284#c26
> > would also fix the problem, and we just unnecessary overcomplicate allocator.
> 
> Is that not the same idea Guy and Jeff discussed that earlier in the
> bug (comments 2 through 6)?

2-4? For me comment #3 is invalid (sorry Jeff :>),
packet_list_dissect_and_cache_record is using own edt structure, and
cfile has other one.

So if cfile->edt->pi.data_src and name will be g_malloced I don't see a problem.

Don't have time to write proper patch, but I think about smth like this:

epan/packet.c
@@ -206,22 +206,16 @@ add_new_data_source(packet_info *pinfo, tvbuff_t *tvb, const char *name)
 {
        data_source *src;

-       src = ep_alloc(sizeof (data_source));
+       src = g_malloc(sizeof (data_source));
        src->tvb = tvb;
-       src->name_initialized = FALSE;
-       src->name = name;
+       src->name = g_strdup(name);
        pinfo->data_src = g_slist_append(pinfo->data_src, src);
 }

 const char*
 get_data_source_name(data_source *src)
 {
-       if (!src->name_initialized) {
-               src->name = ep_strdup_printf("%s (%u bytes)", src->name, tvb_length(src->tvb));
-               src->name_initialized = TRUE;
-       }
-
-       return src->name;
+       return ep_strdup_printf("%s (%u bytes)", src->name, tvb_length(src->tvb));
 }

 /*
@@ -231,6 +225,7 @@ void
 free_data_sources(packet_info *pinfo)
 {
        if (pinfo->data_src) {
+               /* XXX, g_free src->name, src */
                g_slist_free(pinfo->data_src);
                pinfo->data_src = NULL;
        }


We also need to check if Qt allocate own copy of get_data_source_name(). Gtk AFAIR does.