On 7/29/05, LEGO <luis.ontanon@xxxxxxxxx> wrote:
> 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
While it is good to have an api that is orthogonal, while i have been looking at
tvb_get_string() uses virtually every single use is very temporary in scope.
I.e. grab the string, process or display it and then immediately free it.
Would it be useful to add two such functions
tvb_get_ephemeral_string() and
tvb_get_sessional_string()
considering that there will be virtually no users of the second one?
Instead of just a single one tvb_get_string() that is essentially
tvb_get_ephemeral_string() ?
Ok, I have no strong feelings either way.
So :
tvb_get_ephemeral_string() + tvb_get_sessional_string()
tvb_get_ephemeral_faked_unicode() + ...
or any better suggestions?
>
> 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
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>
