Wireshark-dev: Re: [Wireshark-dev] [PATCH] Improved support for MIPv4

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Wed, 16 May 2007 19:17:03 +0200 (CEST)
Hi,

Some additional notes:

+  {REGISTRATION_REVOCATION, "Registration Revocation"},
+  {REGISTRATION_REVOCATION, "Registration Revocation Acknowledgement"},

The second one misses _ACKNOWLEDGEMENT in the symbol

   {0, NULL},
 };

Don't put a comma after the last initializer. It's just poor style.

Thanx,
Jaap

On Wed, 16 May 2007, Sebastien Tandel wrote:

> Hi,
>
> Some quick comments :
>     1) please use proto_tree_add_item whenever possible. Don't use
> tvb_get_* if you don't intend to use the retrieved value for another
> purpose than inserting it in the tree.
>     2) Why have you changed the length type of some fields
> (icmp.mip.flags, icmp.mip.r, icmp.mip.h, ...)? Do we really want to
> be unable to parse correctly mip captures which contains these fields
> but of previous lengths?
> 	Whatever the answer is, please propagate the changes when inserting
> in the tree. At least icmp.mip.flags is inserted once in the tree
> with a display length field of 1.
>
> Some questions :
>     - Do you have traces you can share? If so, can you put them on
> the captures wiki page?
>     - Is there a description on the wiki page?
>     - Have you fuzz tested your changes?
>
>
> Regards,
> Sebastien Tandel
>
> On 16 May 2007, at 05:21, Ville Nuorvala wrote:
>
> > Hello,
> >
> > attached is a patch that adds support for the following RFCs (and
> > RFC-to-be):
> >
> > RFC 3519 Mobile IP Traversal of Network Address Translation (NAT)
> > Devices
> >
> > RFC 3543 Registration Revocation in Mobile IPv4
> >
> > RFC 4433 Mobile IPv4 Dynamic Home Agent (HA) Assignment (including the
> > not yet published errata about the message extension being using the
> > short message extension format)
> >
> > draft-ietf-mip4-message-string-ext Mobile IPv4 Message String
> > Extension
>
> _______________________________________________
> Wireshark-dev mailing list
> Wireshark-dev@xxxxxxxxxxxxx
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
>
>