I think this seems like a good plan.
Can I suggest to call the flag FT_SYNTHETIC instead of FT_GENERATED?
Three things to aware of are
1, the FT_xxx fields are used not only in the hf definitions, they are used
in some stats collection functions, some
helper functions and some dissector functions as well.
All these places must be updated as soon as this change occurs or they will
break.
Only the references in the hf definitions can be done one by one at a later
time.
2, the hidden flag will not be able to solve all issues at all places, that
needs to be worked out first.
I am thinking of when a field is sometimes visible in the tree and at other
instances the very same field is hidden.
Example: nfs.fh.hash IS visible for the PDU in the NFS/NLM/MOUNT/NFSACL
Request/Response pair
where this field occurs. With suitable preference options this field is
ALSO added to the other PDU in the pair
so that useful filters like nfs.fh.hash==0x12345678 will find BOTH PDUs for
those transactions where this file occurs.
This applies to other fields as well. And even more fields in the future.
(smb.fid matching BOTH Request/Reply anyone?)
3, the synthetic flag will not solve all instances either.
Think the nfs.procedure_v3 field.
This field exists and describes in the ONC-RPC NFS requests.
The procedure number does NOT exist in the response PDU but we still want to
display this field in those PDUs anyway.
----- Original Message -----
From: "Biot Olivier"
Sent: Friday, April 30, 2004 9:29 PM
Subject: RE: [Ethereal-dev] [RFC] specially mark protocol fields "generated"
byEthereal?
> |From: Ulf Lamping
> |
> |> Olivier Biot schrieb:
> |>
> |> Also note that fields may have the hidden flag and the
> |generated flag on at
> |> the same time.
> |
> |Of course this should be a flag or'ed to the "normal" FT_
> |field, FTF_HIDDEN and FTF_GENERATED (usually, if hidden is
> |used, generated will also be used), so for example:
> |
> |#define FTF_HIDDEN 0x8000
> |#define FTF_GENERATED 0x4000
> |
> |>
> |> |So the hf_register_info entry could look, like: FT_FRAMENUM |
> |> |FT_GENERATED, or we could provide another set of field registration
> |> |functions, like: proto_tree_add_uint_generated()
> |>
> |> Here we need to be careful as today the FT_* are an enum
> |which may be just
> |> one single 8-bit byte.
> |
> |an enum is an int (ANSI C), and an int is an all of our
> |platforms 32bit AFAIK, so no problem here.
>
> AFAIK int used to be the most common integer type available on a platform;
> it is only guaranteed to be at least 16 bit wide. We must also take care
of
> unwanted sign extension. Maybe restricting to the 15 least significant
bits
> is the safest. In that case, I'd suggest 0x4000 and 0x2000 to be used.
>
> Extracting the field type from the int should be done with a preprocessor
> macro for efficiency reasons, like:
>
> #define FTYPE(x) ((x) & 0x00FF)
> #define FT_IS_GENERATED(x) ((x) & FTF_GENERATED)
> #define FT_IS_HIDDEN(x) ((x) & FTF_HIDDEN)
>
> All the switch(), if and g_assert() calls need then to use the correct
value
> (FTYPE(x) instead of x).
>
> |
> |>
> |> We however still have some spare room in the HFILL macro to
> |accommodate a
> |> bitfield without requiring us to edit all dissectors. We
> |could even write
> |> HFILL for default, HFILL_HIDDEN for hidden, HFILL_GENERATED
> |for generated
> |> and HFILL_GENERATED_HIDDEN for hidden generated fields, but there are
> |> certainly other worthwile approaches.
> |
> |As described above, or'ing to the FT_ fields should be ok for
> |the reasons described above.
>
> Unless someone objects to this by arguing about the maintainability of the
> code? Mmh... They would already have done so, I think :)
>
> |> |Anyone an idea of how much effort that would be?
> |>
> |> Add the basic framework to the field registration: minimal.
> |> Add the field rendering changes: minimal, mainly GTK+.
> |
> |As I know GTK now, both agreed.
> |
> |> Review the dissectors: potentially huge, but not required
> |for technically
> |> running Ethereal.
> |
> |Fortunately, we can make the changes step by step.
>
> And that means we can give it a start very quickly!
>
> Regards,
>
> Olivier
>
