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 > > >