Last night I was looking at how the IP flags are represented, and
noticed that they are displayed in a bitfield that is separate from
the fragmentation offset. They also don't take advantage of the
bit-masking and shifting that the proto_tree routines can do.
Here's a diff showing another way to implement the IP flags.
The programming is clearer, since the 2-byte Flags/Frag-offset value
is passed to the proto_tree routines and _it_ worries about
bitmasking and shifting.
However, I don't work with the IP layer too much. Would this change
bother anybody or seem unnatural to their way of thinking about IP fields?
--gilbert
Index: packet-ip.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-ip.c,v
retrieving revision 1.63
diff -u -r1.63 packet-ip.c
--- packet-ip.c 1999/11/17 01:35:57 1.63
+++ packet-ip.c 1999/11/19 19:02:41
@@ -756,7 +756,6 @@
proto_item *ti, *tf;
gchar tos_str[32];
guint hlen, optlen, len;
- guint16 flags;
int advance;
guint8 nxt;
@@ -853,14 +852,14 @@
proto_tree_add_item(ip_tree, hf_ip_len, offset + 2, 2, iph.ip_len);
proto_tree_add_item(ip_tree, hf_ip_id, offset + 4, 2, iph.ip_id);
- flags = (iph.ip_off & (IP_DF|IP_MF)) >> 12;
- tf = proto_tree_add_item(ip_tree, hf_ip_flags, offset + 6, 1, flags);
+ /* IP Flags and Fragmentation Offset */
+ tf = proto_tree_add_item(ip_tree, hf_ip_flags, offset + 6, 2, iph.ip_off);
field_tree = proto_item_add_subtree(tf, ett_ip_off);
- proto_tree_add_item(field_tree, hf_ip_flags_df, offset + 6, 1, flags),
- proto_tree_add_item(field_tree, hf_ip_flags_mf, offset + 6, 1, flags),
- proto_tree_add_item(ip_tree, hf_ip_frag_offset, offset + 6, 2,
- iph.ip_off & IP_OFFSET);
+ proto_tree_add_item(field_tree, hf_ip_flags_df, offset + 6, 2, iph.ip_off),
+ proto_tree_add_item(field_tree, hf_ip_flags_mf, offset + 6, 2, iph.ip_off),
+ proto_tree_add_item(field_tree, hf_ip_frag_offset, offset + 6, 2, iph.ip_off);
+
proto_tree_add_item(ip_tree, hf_ip_ttl, offset + 8, 1, iph.ip_ttl);
proto_tree_add_item_format(ip_tree, hf_ip_proto, offset + 9, 1, iph.ip_p,
"Protocol: %s (0x%02x)", ipprotostr(iph.ip_p), iph.ip_p);
@@ -1361,16 +1360,17 @@
"" }},
{ &hf_ip_flags_df,
- { "Don't fragment", "ip.flags.df", FT_BOOLEAN, 4, TFS(&flags_set_truth), IP_DF>>12,
- "" }},
+ { "Don't fragment", "ip.flags.df", FT_BOOLEAN, 16, TFS(&flags_set_truth), IP_DF,
+ "If set, prohibits fragmentation" }},
{ &hf_ip_flags_mf,
- { "More fragments", "ip.flags.mf", FT_BOOLEAN, 4, TFS(&flags_set_truth), IP_MF>>12,
- "" }},
+ { "More fragments", "ip.flags.mf", FT_BOOLEAN, 16, TFS(&flags_set_truth), IP_MF,
+ "More IP fragments follow" }},
{ &hf_ip_frag_offset,
- { "Fragment offset", "ip.frag_offset", FT_UINT16, BASE_DEC, NULL, 0x0,
- "" }},
+ { "Fragment offset", "ip.frag_offset", FT_UINT16, BASE_DEC, NULL, IP_OFFSET,
+ "Indicates where in the datagram this fragment belongs, "
+ "measured in 64-bit units." }},
{ &hf_ip_ttl,
{ "Time to live", "ip.ttl", FT_UINT8, BASE_DEC, NULL, 0x0,
