Hi Murphy.
 TLDR: addresses.py should be rerwitten from scratch

I come up against a following problem:
cd pox/lib && python
>>> import addresses
>>> print addresses.IPAddr("255.255.255.0") == addresses.IPAddr6("::1")
  ...
  File "addresses.py", line 634, in __cmp__
    return -cmp(other,self)
  File "addresses.py", line 634, in __cmp__
    return -cmp(other,self)
  File "addresses.py", line 634, in __cmp__
    return -cmp(other,self)
  File "addresses.py", line 634, in __cmp__
    return -cmp(other,self)
RuntimeError: maximum recursion depth exceeded while calling a Python object

In general, there might be an easy fix but  __cmp__ is actually deprecated
by Python3. And, does it really make sense to have ordering on IP
addresses? I mean, implementing equality should be sufficient and whoever
wants to do something fancy (sort IP addresses, can just use to_raw() as a
key)

Moreover, when I was looking at the whole source code, I realized it is
really a huge mess. I think the whole file should be rewritten and maybe be
more strict about api (right now, IPAddr6 can take arguments in so many
different formats that the constructor is one big if-then maze. Similarly,
there are many redundant accessors (e.g. toRaw(),  @property raw,
self._value) as well as two competing style guides (looks like functions
are camelCaps and sometimes lower_case)
Not to mention that the constructors do big amount of magic which is
completely undocumented.

And there are obviously broken parts such as __init__() returning something
else than None
>>> import addresses
>>> addresses.IPAddr6()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() should return None, not 'IPAddr6'

Similarly, converting MAC addresses to IPV6 (IPAddr6.set_mac() function)
seems to be also quite hack-ish and I see no normal reason why this should
be a part of an api

Finally, running python addresses.py crashes with RuntimeError: Host part
of CIDR address is not zero (['1.1.168.103', '255.255.255.0'])

So, to sum up -- I suggest complete rewrite of the file which will
hopefully end up with a cleaner API and simpler code.

Peter

Reply via email to