BIND 10 #353: Move function 'check_port()' and 'check_addr()' to one library

[BIND 10 Development]

#353: Move function 'check_port()' and 'check_addr()' to one library
-------------------------------+--------------------------------------------
      Reporter:  zhanglikun    |        Owner:  jinmei   
          Type:  enhancement   |       Status:  reviewing
      Priority:  major         |    Milestone:           
     Component:  Unclassified  |   Resolution:           
      Keywords:                |    Sensitive:  0        
Estimatedhours:  0.0           |        Hours:  0        
      Billable:  1             |   Totalhours:  0        
      Internal:  0             |  
-------------------------------+--------------------------------------------
Changes (by vorner):

  * owner:  vorner => jinmei


Comment:

 Replying to [comment:5 jinmei]:
 > I've not seen anything obviously wrong, but one test failed for me:
 > [...]
 > This is strange.  Maybe the inet_ntop() implementation on MacOS X (it's
 BSD-derived as far as I know) is buggy.  But we'll have to work around it.

 I modified the function to guess first by a regular expression which
 version of address it is. This saves one call to inet_ntop() that fails in
 case of IPv6 address and should workaround about this problem as a
 sideeffect. Could you run the tests once more and see if it really does? I
 don't have MacOS.

 > I also have following minor comments.  I'd leave it to you whether and
 how to address these points.
 >  - port_check()/addr_check() may not be a good name (and in that sense
 check.py, too) because xxx_check() sounds like performing a check only and
 it's not intuitive for it to return something

 I changed it to _parse, which can be expected to complain if it is
 invalid. Does it seem better?
 >  - it's not clear that IPAddr() raises socket.error against an invalid
 input (unless you read the source code, and has knowledge that inet_pton()
 raises that exception on failure).  Please at least describe it in the
 function description, and it's probably better to use a user-defined
 specific exception.

 Added and documented.

 >  - in check_test.py, you assume specific output of inet_ntop(), but as
 far as I know it's implementation dependent (although we may expect a
 "canonical form" with future implementation due to RFC5952).

 I left it there. For one, I do not see any simple way to check it works.
 For another, even if it is implementation dependent, the canonical form
 (which I hope I put there) seems the only reasonable thing it could return
 and I guess there's no system that would return something else. If there
 is, the only thing that happens would be a false positive of the test (it
 would fail even when it shouldn't), which is better than the other way
 around. Would you mind if it is left there until we actually find a system
 that returns something else?

-- 
Ticket URL: <http://bind10.isc.org/ticket/353#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development