Re: Question regarding emem -> wmem conversion

[Evan Huus <eapache () gmail com>]

On Thu, Sep 12, 2013 at 11:12 AM, Joerg Mayer <jmayer () loplof de> wrote:

> I just looked at packet-tacacs.c. The conversion looked straight forward,
> the em_alloc was easy to convert. Then I noticed calls to
> ep_address_to_str,
> which is done in epan/ep_address_to_str.c
> Is it OK to leave that and only convert the other strings, i.e. is it OK
> if some of the memory is handled by emem and some by wmem?

That's fine. You obviously can't wmem_free memory allocated by emem, but
they can be used together in the same dissector otherwise. Once I've gotten
through all the direct calls in the dissector I plan to start looking at
the indirect calls (like ep_address_to_str and friends) but they are more
complicated: they are not always called just when a packet is being
dissected, so their actual scope (since it is not packet scope) is unclear.

The only other thing to be aware of when doing conversions is that user
table callbacks (UATs) sometimes use ep_ memory, but can be called from the
preferences page when no packet is being dissected, so if you convert these
to wmem they will assert. UAT memory management is also on my list to
figure out once the main conversions are done.

Thanks for your help with the conversions!
Evan

P.S. I have a few vim macros that I wrote to speed up the process. I can
share them if anybody using vim wants to help :)

Thanks
>     Jörg
>
>
> Index: dissectors/packet-tacacs.c
> ===================================================================
> --- dissectors/packet-tacacs.c  (revision 51969)
> +++ dissectors/packet-tacacs.c  (working copy)
> @@ -63,7 +63,7 @@
>
>  #include <epan/packet.h>
>  #include <epan/prefs.h>
> -#include <epan/emem.h>
> +#include <epan/wmem/wmem.h>
>  #include <epan/expert.h>
>  #include "packet-tacacs.h"
>
> @@ -1287,14 +1287,14 @@
>         int i,j;
>         size_t md5_len;
>         md5_byte_t *md5_buff;
> -       md5_byte_t hash[MD5_LEN];                                       /*
> the md5 hash */
> +       md5_byte_t hash[MD5_LEN];                               /* the md5
> hash */
>         md5_byte_t *mdp;
>         md5_state_t mdcontext;
>
>         md5_len = 4 /* sizeof(session_id) */ + strlen(key)
>                         + sizeof(version) + sizeof(seq_no);
>
> -       md5_buff = (md5_byte_t*)ep_alloc(md5_len+MD5_LEN);
> +       md5_buff = (md5_byte_t*)wmem_alloc(wmem_packet_scope(),
> md5_len+MD5_LEN);
>
>
>         mdp = md5_buff;
> --
> Joerg Mayer                                           <jmayer () loplof de>
> We are stuck with technology when what we really want is just stuff that
> works. Some say that should read Microsoft instead of technology.
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request () wireshark org
> ?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe