Ethereal-dev: Re: [Ethereal-dev] ememification of tvb_get_tring() and friends

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

Date: Fri, 29 Jul 2005 00:57:04 +0200
Ephemeral, it is a little too long but IMHO it gets right to the point.

ephemeral_alloc()
ephemeral_memdup()
ephemeral_strdup()
ephemeral_strndup()
ephemeral_strdup_printf()
tvb_get_ephemeral_string()
tvb_ephemeral_memdup()


While for the per session ones I thought in "sessional" that is
something in between seasonal and session :-P

Luis


On 7/29/05, Ulf Lamping <ulf.lamping@xxxxxx> wrote:
> ronnie sahlberg wrote:
> 
> >List
> >
> >during my crusade to ememify the tvb_get_string() (and friends such as
> >tvb_fake_unicode(), tvb_get_text(), tvb_get_stringz())
> >
> >
> Hmmm, I found the function names chosen are very unintuitive at all. I
> wouldn't know without reading the docs the difference between these
> functions. And even worse the name tvb_fake_unicode() seems to be
> completely odd in this row (at least to me). Should this be something
> like tvb_get_faked_unicode() ?
> 
> >i have found several obvious memleaks (and a lot more nonobvious like
> >where an exception is thrown between where tvb_get_string() is called
> >and before g_free()) which is good.
> >
> >
> Handling the memleaks caused by exceptions isn't really obvious.
> However, as it's name implies it should be an exception ...
> 
> And yes, preventing problems like this is a good thing :-)
> 
> >Something that would imho be good since the number of both obvious and
> >non-obvious memleaks for these calls show that the api in using them
> >is too complex.
> >
> >
> No. The problem is not complexity of the functions (their job is quite
> simple) but simply wrong *naming* of the function(s) is the problem. The
> function named tvb_get_string() gives no real hint that you have to free
> the string by yourself afterwards.
> 
> As an opposite, the g_strdup named functions from glib (although glib is
> itself a bit inconsistent in naming) makes it pretty clear that
> something is duplicated and gives at least a hint to take care about the
> function output. So if we had used something like tvb_strdup() would
> have probably reduced our problems a lot on this topic. Of course, this
> function name would be misleading with the new functionality you are
> going to provide.
> 
> And again: Never underestimate the power of function and variable naming
> to avoid such problems.
> 
> On the other hand, having at least four functions to get a string from a
> tvb maybe proof you're probably right that the API is complex but for a
> different reason :-)
> 
> >it would probably also speed up ethereal a tiny tiny fraction reducing
> >the amount of g_free() calls made but that is likely insignificant.
> >
> >
> >
> I would think the performance gain will be minimal, but a step in the
> right direction and better than nothing ;-)
> 
> >once this is finished i plan not to have any explicit
> >ep_tvb_get_string() calls anymore. only the tvb_get_string() and
> >friends but with a different semantic on how/when memory is freed.
> >Since so very few functions actually need tvb_get_string() data with
> >longer than to the end of dissecting the current packet semantics
> >those other very few users can just do their own g_strdup()/g_free()
> >  or when added ec_alloc() call (like ep_alloc() but with lifetime
> >until new capture is loaded).
> >
> >
> >comments?
> >
> >
> The problem seems to be common and your solution to it seems reasonable.
> I'm happy that someone takes a look at these things.
> 
> BTW: One could expect that the string will last as long as the tvb
> exists. This was an idea I had when reading your mail. Why not attach
> the strings to the tvb and free them when the tvb is freed? This seems
> to be more obvious than your idea (at least to me) ... However, it might
> be easier to implement that all memory blocks are related to the packet
> dissection and freed afterwards as you've described your solution, so
> there's no real difference in the two possibilities, so simply chose the
> easier to implement one :-)
> 
> And what about changing the names of the functions to something that
> makes it more obvious that the string will be gone after the packet was
> processed?
> 
> I don't have a good idea about this naming, maybe something like:
> transient, temp or alike, so this functions named like:
> tvb_get_transient_string(), however, there must be a better name than my
> suggestions ...
> 
> Regards, ULFL
> 
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
> 


-- 
This information is top security. When you have read it, destroy yourself.
-- Marshall McLuhan