On Thu, Mar 28, 2013 at 7:56 PM, Guy Harris <guy@xxxxxxxxxxxx> wrote:
>
> On Mar 28, 2013, at 3:10 PM, Evan Huus <eapache@xxxxxxxxx> wrote:
>
>> Tangential to this, does anybody know what the deal is with
>> decode_enumerated_bitfield() and decode_enumerated_bitfield_shifted()?
>
> The deal may be that...
>
>> The first is called in exactly one place, the second not at all.
>
> ...the first was used more in the past (which it was), but uses got replaced with named fields. and maybe the second was used but replaced by named fields as well.
>
> They were helper routines to use with proto_tree_add_text().
>
>> They return static buffers (which is odd, though not necessarily wrong,
>> perhaps they should be using packet-scope memory?),
>
> At the time they were created, packet-scope memory didn't exist. They're *really* old, as in "before we renamed it to Wireshark" old.
>
>> and while they
>> make use of value strings they don't seem immediately value-string
>> related.
>
> I guess if they belong anywhere, it's in to_str.c along with the other decode_XXX_bitfield routines.
>
>> The only one that is called is fairly short so I'm tempted to manually
>> inline that and drop both functions. At the very least they should
>> probably be moved to to_str.c (or somewhere else).
>>
>> Thoughts?
>
> Fix dissect_nfs_fattr4_fh_expire_type() to use named fields and proto_tree_add_item(), and then get rid of both decode_enumerated_bitfield() and decode_enumerated_bitfield_shifted()?
Agreed. I've added that to the list of misc. cleanups for bug 8467.