Ethereal-dev: [Ethereal-dev] Re: rev 15660: /trunk/plugins/asn1/: packet-asn1.c /trunk/epan/:
Oh,
One more reason to refactor the GStrings away is to reduce the
dependencies in dissectors in glib
which might or might not be something we want to do given that it is
high time to autid all the dissectors anyway.
Given that this refactoring and auditing should be done anyway, the
Glib undependency works would be a near zero-cost operation.
One remaining important structure we still use from glib is the
GHashTable that I very strongly belive has performance issues,
possibly due to the fact it is very difficult/impossible to find
documentation and information on how to select and distribute the hash
values we calculate for optimal performance.
I think that for GHashTables we should in the future implement and
develop a replacement that use identical signatures to the current
New/Destroy/Insert/Remove/Lookup apis and replace it with our own
implementation based on splice-trees.
While splice-trees do not self-balance as nicely as red-black trees I
think it would be preferable for us to use splice-trees due to the way
"normal" captures work and that lookup of "common" entries are
optimized to be fast.
On 9/2/05, ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
> On 9/1/05, Guy Harris <gharris@xxxxxxxxx> wrote:
> > Ulf Lamping wrote:
> > > sahlberg@xxxxxxxxxxxx wrote:
> > > g_snprintf(str, 128-(str-string), "%s%u", prepend_comma?",":"",
> > >
> > > The expression "128-(str-string)" is exactly which should be avoided!!!
> > >
> > > This way of coding is *very* error intensive, at least when changes are
> > > made to the sources later.
> >
> > Yes, it's a somewhat awkward idiom, possibly leaving more room for
> errors.
>
> Yes you are right. It is suboptimal.
>
>
> >
> > It also means the dissector imposes a limit on the length of the string.
> >
> > > Maybe copying the GString code from GLib and replace the g_malloc calls
> > > by ep_malloc (well and change the prefix g_string_ to something like
> > > ep_string_ or course)?
> >
> > Or, alternatively, implementing an emem-based asprintf-style API, perhaps
> > using g_printf_string_upper_bound()?
> >
> > Another possibility is to use proto_item_append_text() - put the initial
> > information into an item created with a proto_tree_add_ routine, and then
> > append to it.
> >
>
> Good point.
>
> There are only very very few dissectors thus far that do this kind of
> string manipulation.
> Most dissectors dont do any string manipulation at all.
>
>
>
> I think that in a future audit phase one should go through all string
> processing and convert as many as possible into
> standard value_strings (some dissectors still use switch-statements to
> pick out static strings) and append the strings to col_info and the
> proto_item one component at a time instead of first building these
> temporary strings.
>
>
> My aim was to first get rid of all sprintf() which was done a week
> ago, and then all GString and g_string_...
> and standardize on g_snprintf() for this phase.
>
>
> I also audited that all value_strings were terminated properly.
>
>
> Next phase i would like to eliminate all strcat and strcpy.
>
>
>
> In a future phase i would like to eliminate all character arrays that
> are either global variables or stack variables to become either sp or
> ep emem allocated memory.
> In particular i would like to move every one of them off the stack
> since this would reduce the danger of buffer overflows.
> eliminating all use of arrays that live on the stack would virtually
> eliminate the danger of buffer overflows as an attack vector.
> eliminating all global/static arrays would reduce the runtime
> requirement of memory for ethereal slightly.
>
>
> After that i think it would be nice to go over all loops and verify
> that offset is always increased in each iteration or that the loop
> terminates with an exception.
>
>
> After that i think we should be in pretty decent shape, at least
> better shape than we were in in 0.10.11
>
