Yevgeny Zaspitsky has posted comments on this change.

Change subject: tools: CidrValidation Utils
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.ovirt.org/#/c/32539/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/validation/CidrValidator.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/validation/CidrValidator.java:

Line 41:     private static int covnertIpToInt(String ipAdd) {
Line 42:         String[] subAdd = ipAdd.split("\\.");
Line 43:         int output = 0;
Line 44:         int temp;
Line 45:         for (int index = 3; index > -1; index--) {
> 1. OutOfBoundaries prone.
i'd prefer running from 0 to 3
Line 46:             temp = Integer.parseInt(subAdd[3 - index]);
Line 47:             temp <<= (index * 8);
Line 48:             output |= temp;
Line 49:         }


Line 50: 
Line 51:         return output;
Line 52:     }
Line 53: 
Line 54:     private static boolean isNetworkAddress(int ip, int mask) {
> i'd call it isIpBelongToSubnet
sorry for bothering. I didn't guess well the intention
Line 55:         int temp = 1;
Line 56:         for (int i = 0; i < 32-mask; i++) {
Line 57:             if ((temp & ip) != 0) {
Line 58:                 return false;


Line 52:     }
Line 53: 
Line 54:     private static boolean isNetworkAddress(int ip, int mask) {
Line 55:         int temp = 1;
Line 56:         for (int i = 0; i < 32-mask; i++) {
> can the check be done without a loop?
the new version of the method according to the requirements:

    private boolean isSubnetAddress(final int ipAsInt, final int subnetBits) {
        final int mask = (0xffffffff << (32 - subnetBits)) & ipAsInt;
        return (ipAsInt ^ mask) == 0;
    }
Line 57:             if ((temp & ip) != 0) {
Line 58:                 return false;
Line 59:             }
Line 60: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1277dbc815953926fe1648350cd55cb75e1084a
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to