Re: RFC: sorted value_string + bsearch

[Anders Broman <a.broman () telia com>]

Jakub Zawadzki skrev 2010-04-23 16:54:
> On Fri, Apr 23, 2010 at 04:18:40PM +0200, Anders Broman wrote:
>    
> > +const gchar*
> > +match_strval_fast(const guint32 val, const value_string_fast *vs) {
> > +  guint low, idx, max;
> > +  guint32 item;
> > +  if(vs) {
> > +       switch(vs->match_type){
> > +               case VS_SEARCH:
> > +                       match_strval( val, vs->vals);
> > +                       break;
> > +               case VS_INDEX:
> > +                       return vs->vals[val].strptr;
> >      
> I think you need:
>       return (val<  vs->length) ? vs->vals[val].strptr : NULL;
>    
Yes some polishing needed :-)
> anyway sorry, but I dislike your idea :)
>
> I would prefer other function or even macro:
>
> #define match_strval_hyper_fast(val, vs) \
>       (val>= 0&&  val<  array_length(vs)&&  vs[val].val == val) ? \
>               vs[val].strptr : match_strval(val, vs) /* fallback (?) */
>
>    
I think we will get the most out of this trough proto_add_item() and 
friends which means
using a new mask for each "matching" function in the display "variable" 
hence it may be better to
indicate which "fast" option to use in the struct we may also come up 
with others as we go trough the dissectors, like a full
table starting at a number ( like 500 - 1000 ->value  match index minus 
offset).
/Anders
>    
> > > I think this is a good idea, but it obviously requires the value string  array to be properly sorted for it to work 
> > > well.
> > > And the bigger the array, the more enticing it would be to use this faster method, but I think it will also be 
> > > potentially harder to ensure that those bigger arrays are indeed properly sorted.
> > > Developers are bound to make mistakes, especially with the very large arrays, and as other developers add new 
> > > entries, they may not know that a particular array is supposed to be sorted, and even if they do, there's no 
> > > guarantee that a properly sorted array will remain so.
> > >        
> Can't we do this check in tmp_fld_check_assert()?
> I think most of value_strings are registered...
> ___________________________________________________________________________
> 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