Hi guys,

We are working with FindBugs and trying to get Scariest and Scary bugs fixed on 
the master branch.

FindBugs has reported a bug on the contrail plugin, in the VirtualMachineModel 
class. If you check the Checking the commit id 
cc2b1c4961244d9c3d8b452f1dcaa6614e56d11a, you will see the following:

        ServiceInstance siObj;
       try {
            siObj = (ServiceInstance) api.findById(ServiceInstance.class, 
serviceUuid);
        } catch (IOException ex) {
            s_logger.warn("service-instance read", ex);
            throw new CloudRuntimeException("Unable to read service-instance 
object", ex);
        }

        ServiceInstanceModel siModel;
        if (siObj == null) {
            siModel = new ServiceInstanceModel(serviceUuid);
            siModel.build(controller, siObj);
            manager.getDatabase().getServiceInstances().add(siModel);
        } else {
            String fqn = StringUtils.join(siObj.getQualifiedName(), ':');
            siModel = manager.getDatabase().lookupServiceInstance(fqn);
            if (siModel == null) {
                siModel = new ServiceInstanceModel(serviceUuid);
                siModel.build(controller, siObj);
                manager.getDatabase().getServiceInstances().add(siModel);
            }
        }

If the ServiceInstance object (siObj) is null after the call to api.findById, 
the following call to siModel.buil() will break really bad:

::: ServiceInstanceModel:::build() method

    public void build(ModelController controller, ServiceInstance siObj) {
        ApiConnector api = controller.getApiAccessor();
        _serviceInstance = siObj;
        _fqName = StringUtils.join(siObj.getQualifiedName(), ':');
        ServiceInstanceType props = siObj.getProperties();
        // TODO: read management network names and cache network objects.
        ObjectReference ref = siObj.getServiceTemplate().get(0);

As you can see in the snippet above, the ServiceInstance siObj is used in 4 
places, whereas we will face a NPE in the getQualifiedName() method.

I would suggest that in the VirtualMachineModel.build() method we check for 
empty/valid UUID before calling the private method (buildServiceInstance). At 
this moment the build() method only checks if the UUID is not null.

Looking forward to your response.

With kind regards,
Wilder

Reply via email to