True, as a part of logging infrastructure the last version is preferred,
will do that...

Peter
(sent from android device)
On Jul 23, 2014 8:34 PM, "Murphy McCauley" <murphy.mccau...@gmail.com>
wrote:

> This looks good to me, though I wonder if you'd be willing to do a little
> more while you're at it.  Good POX code should always use a literal tuple
> when using the string formatting operator.  This may not actually be on the
> wiki, but it should be.  The packet library predates POX and its
> conventions, though, so there's plenty of code in there which breaks the
> rule.
>
> So basically, things like...
> self.msg('(ip parse) warning: IP version %u not IPv4' % self.v)
>
> should be...
> self.msg('(ip parse) warning: IP version %u not IPv4' % (self.v,))
>
> Or, better yet, I think the following would work:
> self.msg('(ip parse) warning: IP version %u not IPv4', self.v)
>
> And for that matter, I don't think there's any reason not to do:
> self.msg('(ip parse) warning: IP version %s not IPv4', self.v)
>
> .. which is less likely to have an attempt at a helpful log message result
> in an exception instead.
>
>
> If you don't have the time and inclination, let me know.  Just seemed to
> me like as long as you're fixing things... might as well fix them the rest
> of the way.
>
> (And I'll respond on the subject of addresses.py eventually; just haven't
> quite had the time to yet.)
>
> -- Murphy
>
> On Jul 22, 2014, at 1:05 AM, Peter Peresini <peter.peres...@epfl.ch>
> wrote:
>
> Hi Murphy,
>  here is a small patch that improves warning messages generated during
> IPv4 parsing.
>
> Peter
>
>
> From 7b5f395bd49696290d647b08db271a4ca0e06e43 Mon Sep 17 00:00:00 2001
> From: Peter Peresini <peter.peres...@gmail.com>
> Date: Tue, 22 Jul 2014 10:02:18 +0200
> Subject: [PATCH] Improved warning messages during IPv4 parsing
>
> ---
>  pox/lib/packet/ipv4.py | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/pox/lib/packet/ipv4.py b/pox/lib/packet/ipv4.py
> index 3eb4ae6..6c47a2e 100644
> --- a/pox/lib/packet/ipv4.py
> +++ b/pox/lib/packet/ipv4.py
> @@ -123,19 +123,22 @@ class ipv4(packet_base):
>          self.srcip = IPAddr(self.srcip)
>
>          if self.v != ipv4.IPv4:
> -            self.msg('(ip parse) warning IP version %u not IPv4' % self.v)
> +            self.msg('(ip parse) warning: IP version %u not IPv4' %
> self.v)
>              return
> -        elif self.hl < 5:
> -            self.msg('(ip parse) warning IP header %u longer than len %u'
> \
> -                        % (self.hl, self.iplen))
> +        if self.hl < 5:
> +            self.msg('(ip parse) warning: IP header length shorter than
> MIN_LEN (IHL=%u => header len=%u)' \
> +                        % (self.hl, 4 * self.hl))
>              return
> -        elif self.iplen < ipv4.MIN_LEN:
> -            self.msg('(ip parse) warning invalid IP len %u' % self.iplen)
> +        if self.iplen < ipv4.MIN_LEN:
> +            self.msg('(ip parse) warning: Invalid IP len %u' % self.iplen)
>              return
> -        elif (self.hl * 4) >= self.iplen or (self.hl * 4) > dlen:
> -            self.msg('(ip parse) warning IP header %u longer than len %u'
> \
> +        if (self.hl * 4) > self.iplen:
> +            self.msg('(ip parse) warning: IP header longer than IP length
> including payload (%u vs %u)' \
>                          % (self.hl, self.iplen))
>              return
> +        if (self.hl * 4) > dlen:
> +            self.msg('(ip parse) warning: IP header is truncated')
> +            return
>
>          # At this point, we are reasonably certain that we have an IP
>          # packet
> --
> 1.8.3.2
>
>
>

Reply via email to