Change in vdsm[master]: Improve the validation of netmask and gateway

asegurap at redhat.com asegurap at redhat.com
Wed Sep 5 16:07:44 UTC 2012


Antoni Segura Puimedon has posted comments on this change.

Change subject: Improve the validation of netmask and gateway
......................................................................


Patch Set 2: (2 inline comments)

I really like the improvements to netmask and gateway validation, they are so much more complete and true with this patch. My only gripe is with the abuse of the string formatting in line of the beast(666).

Looking very much forward to +1 this patch.

....................................................
File vdsm/configNetwork.py
Line 662:         return False
Line 663:     return True
Line 664: 
Line 665: def _dottedQuadToNum(ipaddr):
Line 666:     return int(''.join('%02x' % int(i) for i in ipaddr.split('.')), 16)
Might I propose to avoid the string->int->string->int by doing something like:
shift, total = 24, 0
for num in ipadder.split('.'):
    total += int(num) << shift
    shift -= 8
return total.
Line 667: 
Line 668: def validateIpAddress(ipAddr):
Line 669:     if not _validateIpAddress(ipAddr):
Line 670:         raise ConfigNetworkError(ne.ERR_BAD_ADDR, "Bad IP address: %r"%ipAddr)


Line 672: def validateNetmask(netmask):
Line 673:     if not _validateIpAddress(netmask):
Line 674:         raise ConfigNetworkError(ne.ERR_BAD_ADDR, "Bad netmask: %r"%netmask)
Line 675:     num = _dottedQuadToNum(netmask)
Line 676:     if (num & (num - 1)) != (num << 1) & 0xffffffff:
I love this approach.
If you want you can drop the parenthesis though, as the operator precedence makes it work just right ;-) Although I guess that you could keep some for increased readability.
Line 677:         raise ConfigNetworkError(ne.ERR_BAD_ADDR, "Bad netmask: %r"%netmask)
Line 678: 
Line 679: def validateGateway(gateway, ipaddr, netmask):
Line 680:     if not _validateIpAddress(gateway):


--
To view, visit http://gerrit.ovirt.org/7713
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c0a39f3f681a9e587f9118246e42a29e18d604b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list