Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 52264: /trunk/ /trunk/epan/: value_s
From: Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>
Date: Sun, 29 Sep 2013 21:56:22 +0200
On Sun, Sep 29, 2013 at 08:46:23AM -0400, Evan Huus wrote:
> On Sun, Sep 29, 2013 at 8:44 AM,  <eapache@xxxxxxxxxxxxx> wrote:
> > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=52264
> >
> > User: eapache
> > Date: 2013/09/29 12:44 PM
> >
> > Log:

[...]

> >  This is one of the first steps towards converting val_to_str and friends to
> >  wmem. I'm honestly not sure what the best approach is for the API in this case:
> >  the vast majority of usage is within dissectors, so just hard-coding packet
> >  scope (the way they currently hard-code ep_ scope) doesn't look terrible, but
> >  there are *some* uses in taps and other places that will need to be converted to
> >  something else if we go that route. Adding a wmem_pool parameter just for the
> >  uncommon case seems a bit like overkill, though perhaps it is the right thing to
> >  do.
> 
> Thoughts on this?

Most of current calls are done by col_append_fstr() or expert_add_info_format()
so it'd be actually great to have own formatting conversion.
/me smiles to register_printf_function(), still not portable ;|

So... What about removing val_to_str(), val_to_str_const() rename try_val_to_str() to val_to_str()
make caller responsible to do what he want when there's no value for value_string entry.
value_string API don't allocates memory -> no problem ;]


But back to topic (cause you'll probably see this problem few more times).
I don't quite get a point why we need to change everything to wmem.
(To be honest I still don't quite get why we need wmem_ at all, but let's skip that).
But if we really want to do that (change everything to wmem_), we NEED some ep-like temporary pool (which will work both for UI and dissection),
or some function which will return packet-pool or gui-pool if there's no dissection. Otherwise we need to remove some functionality.

Still I don't see any UI-wmem-pool, do we have one?

PS: I know it's obvious, sorry ;|