Ravi Nori has posted comments on this change.

Change subject: restapi: ProductInfo use ProductRPMVersion
......................................................................


Patch Set 3: (2 inline comments)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
Line 378:         return rsdl;
Line 379:     }
Line 380: 
Line 381:     private API addSystemVersion(API api) {
Line 382:         String ProductVersion = getConfigurationValue(String.class, 
ConfigurationValues.ProductRPMVersion, null);
can we make the variable name productVersion with a lower case p
Line 383:         api.setProductInfo(new ProductInfo());
Line 384:         api.getProductInfo().setName("oVirt Engine");
Line 385:         api.getProductInfo().setVendor("ovirt.org");
Line 386:         
api.getProductInfo().setVersion(VersionHelper.parseVersion(ProductVersion));


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VersionHelper.java
Line 15: 
Line 16:         version.setFullVersion(text);
Line 17:         version.setMajor(Integer.parseInt(parts[0]));
Line 18:         version.setMinor(Integer.parseInt(parts[1]));
Line 19: 
If the text is not in the format required this code will throw an 
ArrayIndexOutOfBoundsException or a NumberFormatException . I think we should 
check the array length before accessing it. Something to the effect of the 
following.
And only when major, minor, build and revision are not -1 we should return a 
Version instance.

{
    int major = -1;
    in minor = -1;
    if (parts.length > 0) {
       major = parseInt(parts[0]);
    }

    if (parts.length > 1) {
       minor = parseInt(parts[1]);
    }

....
}

private static int parseInt(String val) {
    try {
        return Integer.parseInt(val);
    } catch (NumberFormatException e) {
        return -1;
    }
}
Line 20:         if (parts[2].contains("-")) {
Line 21:             version.setBuild(Integer.parseInt(parts[2].split("-")[0]));
Line 22:         } else {
Line 23:             version.setBuild(Integer.parseInt(parts[2]));


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5c03460ef1af943278ab6f5171c042afdf4d392
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to