Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 32929 - (ENC_*_ENDIAN vs FI_*_ENDIAN)

[Jakub Zawadzki <darkjames@xxxxxxxxxxxxxxxx>]

Hi,

> -       FI_SET_FLAG(new_fi, (little_endian) ? FI_LITTLE_ENDIAN : FI_BIG_ENDIAN);
> +       /*
> +        * XXX - this should just check the REP_*_ENDIAN bit, with
> +        * those fields for which we treat any non-zero value of
> +        * "encoding" checking the rest of the bits.
> +        */
> +       FI_SET_FLAG(new_fi, (encoding) ? FI_LITTLE_ENDIAN : FI_BIG_ENDIAN);

I'm not sure if I understand this comment properly...

I could instead of using (FI_LITTLE_ENDIAN, FI_BIG_ENDIAN) use (ENC_LITTLE_ENDIAN, ENC_BIG_ENDIAN, ENC_NA),
and do FI_SET_FLAG(new_fi, encoding) (if it is what comment suggest)

but ENC_LITTLE_ENDIAN should have different value than ENC_NA
 (preferably ENC_LITTLE_ENDIAN != 0)

I know there's big problem with it, cause many dissectors are
using encoding as little_endian (with TRUE/FALSE values)

The easiest case when TRUE/FALSE is implicit used:
#v+
 $ grep -Ir '\<ptvcursor_add\>' ./ |egrep 'FALSE|TRUE'
 325
 $ grep -Ir '\<proto_tree_add_item\>' ./ |egrep 'FALSE|TRUE' | wc -l
 16257
 $ grep -Ir '\<ptvcursor_add_no_advance\>' ./ |egrep 'FALSE|TRUE' | wc -l
 90
#v-

Fixing it by hand is impossible...
(I don't like using sed, but some time ago I tried to use Coccinelle [1]
it's working quite nice, but coccinelle is really slow)

It's quite big change so I think we should wait till 1.5 branch,
anyway if we want to make so big change maybe we could put encoding in header_field_info?

(I'm unsure if you want to break API/ABI)

Btw. what about proto_tree_add_bitmask*()? 
It still has: (gboolean little_endian) instead of encoding, do you plan to change it?

[1] http://coccinelle.lip6.fr/