cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Erik Weber <terbol...@gmail.com>
Subject Re: New Defects reported by Coverity Scan for cloudstack
Date Tue, 26 Jan 2016 09:14:25 GMT
My idea was to rule them out during PR reviewal, be it false or positive
issues, rather than $x days/weeks/months later :-)


-- 
Erik

On Tue, Jan 26, 2016 at 9:23 AM, Daan Hoogland <daan.hoogland@gmail.com>
wrote:

> please Erik, if you have any inspiration to this cause and goal...
>
> I'm game if at all feasible. Don't forget we have 6000+ outstanding defects
> as far as coverity is concerned. Not all of those are genuine positives,
> btw.
>
> On Mon, Jan 25, 2016 at 7:16 PM, Erik Weber <terbolous@gmail.com> wrote:
>
> > Slighly off topic, but could coverity be integrated with our PR workflow?
> >
> > Erik
> >
> > Den mandag 25. januar 2016 skrev Daan Hoogland <daan.hoogland@gmail.com>
> > følgende:
> >
> > > Did a quick scan of the reported new issues. All four are older then
> the
> > > last report. Anybody feels like taking ownership?
> > >
> > > On Mon, Jan 25, 2016 at 4:13 PM, <scan-admin@coverity.com
> > <javascript:;>>
> > > wrote:
> > >
> > > >
> > > > Hi,
> > > >
> > > > Please find the latest report on new defect(s) introduced to
> cloudstack
> > > > found with Coverity Scan.
> > > >
> > > > 4 new defect(s) introduced to cloudstack found with Coverity Scan.
> > > >
> > > >
> > > > New defect(s) Reported-by: Coverity Scan
> > > > Showing 4 of 4 defect(s)
> > > >
> > > >
> > > > ** CID 1147055:  Null pointer dereferences  (REVERSE_INULL)
> > > > /server/src/com/cloud/storage/VolumeApiServiceImpl.java: 2204 in
> > > >
> > >
> >
> com.cloud.storage.VolumeApiServiceImpl.extractVolume(org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd)()
> > > >
> > > >
> > > >
> > > >
> > >
> >
> ________________________________________________________________________________________________________
> > > > *** CID 1147055:  Null pointer dereferences  (REVERSE_INULL)
> > > > /server/src/com/cloud/storage/VolumeApiServiceImpl.java: 2204 in
> > > >
> > >
> >
> com.cloud.storage.VolumeApiServiceImpl.extractVolume(org.apache.cloudstack.api.command.user.volume.ExtractVolumeCmd)()
> > > > 2198
> > > > 2199                 VMTemplateVO template =
> > > > ApiDBUtils.findTemplateById(volume.getTemplateId());
> > > > 2200                 if (template != null) { // For ISO based volumes
> > > > template = null and
> > > > 2201                     // we allow extraction of all ISO based
> > > > 2202                     // volumes
> > > > 2203                     boolean isExtractable =
> > template.isExtractable()
> > > > && template.getTemplateType() != Storage.TemplateType.SYSTEM;
> > > > >>>     CID 1147055:  Null pointer dereferences  (REVERSE_INULL)
> > > > >>>     Null-checking "account" suggests that it may be null,
but it
> > has
> > > > already been dereferenced on all paths leading to the check.
> > > > 2204                     if (!isExtractable && account != null
&&
> > > > !_accountMgr.isRootAdmin(account.getId())) {
> > > > 2205                         // Global admins are always allowed to
> > > extract
> > > > 2206                         PermissionDeniedException ex = new
> > > > PermissionDeniedException("The volume with specified volumeId is not
> > > > allowed to be extracted");
> > > > 2207                         ex.addProxyObject(volume.getUuid(),
> > > > "volumeId");
> > > > 2208                         throw ex;
> > > > 2209                     }
> > > >
> > > > ** CID 1192794:    (REVERSE_INULL)
> > > > /server/src/com/cloud/api/query/QueryManagerImpl.java: 2959 in
> > > >
> > >
> >
> com.cloud.api.query.QueryManagerImpl.listDataCentersInternal(org.apache.cloudstack.api.command.user.zone.ListZonesCmd)()
> > > > /server/src/com/cloud/api/query/QueryManagerImpl.java: 2959 in
> > > >
> > >
> >
> com.cloud.api.query.QueryManagerImpl.listDataCentersInternal(org.apache.cloudstack.api.command.user.zone.ListZonesCmd)()
> > > >
> > > >
> > > >
> > > >
> > >
> >
> ________________________________________________________________________________________________________
> > > > *** CID 1192794:    (REVERSE_INULL)
> > > > /server/src/com/cloud/api/query/QueryManagerImpl.java: 2959 in
> > > >
> > >
> >
> com.cloud.api.query.QueryManagerImpl.listDataCentersInternal(org.apache.cloudstack.api.command.user.zone.ListZonesCmd)()
> > > > 2953                     }
> > > > 2954                 }
> > > > 2955
> > > > 2956                 // handle available=FALSE option, only return
> > zones
> > > > with at least
> > > > 2957                 // one VM running there
> > > > 2958                 Boolean available = cmd.isAvailable();
> > > > >>>     CID 1192794:    (REVERSE_INULL)
> > > > >>>     Null-checking "account" suggests that it may be null,
but it
> > has
> > > > already been dereferenced on all paths leading to the check.
> > > > 2959                 if (account != null) {
> > > > 2960                     if ((available != null) &&
> > > > Boolean.FALSE.equals(available)) {
> > > > 2961                         Set<Long> dcIds = new HashSet<Long>();
> //
> > > > data centers with
> > > > 2962                         // at least one VM
> > > > 2963                         // running
> > > > 2964                         List<DomainRouterVO> routers =
> > > > _routerDao.listBy(account.getId());
> > > > /server/src/com/cloud/api/query/QueryManagerImpl.java: 2959 in
> > > >
> > >
> >
> com.cloud.api.query.QueryManagerImpl.listDataCentersInternal(org.apache.cloudstack.api.command.user.zone.ListZonesCmd)()
> > > > 2953                     }
> > > > 2954                 }
> > > > 2955
> > > > 2956                 // handle available=FALSE option, only return
> > zones
> > > > with at least
> > > > 2957                 // one VM running there
> > > > 2958                 Boolean available = cmd.isAvailable();
> > > > >>>     CID 1192794:    (REVERSE_INULL)
> > > > >>>     Null-checking "account" suggests that it may be null,
but it
> > has
> > > > already been dereferenced on all paths leading to the check.
> > > > 2959                 if (account != null) {
> > > > 2960                     if ((available != null) &&
> > > > Boolean.FALSE.equals(available)) {
> > > > 2961                         Set<Long> dcIds = new HashSet<Long>();
> //
> > > > data centers with
> > > > 2962                         // at least one VM
> > > > 2963                         // running
> > > > 2964                         List<DomainRouterVO> routers =
> > > > _routerDao.listBy(account.getId());
> > > >
> > > > ** CID 1349986:  FindBugs: Dodgy code  (FB.DLS_DEAD_LOCAL_STORE)
> > > >
> > >
> >
> /plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageSendCommandWrapper.java:
> > > > 76 in
> > > >
> > >
> >
> com.cloud.hypervisor.xenserver.resource.wrapper.xen610.XenServer610MigrateWithStorageSendCommandWrapper.execute(com.cloud.agent.api.MigrateWithStorageSendCommand,
> > > > com.cloud.hypervisor.xenserver.resource.XenServer610Resource)()
> > > >
> > > >
> > > >
> > > >
> > >
> >
> ________________________________________________________________________________________________________
> > > > *** CID 1349986:  FindBugs: Dodgy code  (FB.DLS_DEAD_LOCAL_STORE)
> > > >
> > >
> >
> /plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageSendCommandWrapper.java:
> > > > 76 in
> > > >
> > >
> >
> com.cloud.hypervisor.xenserver.resource.wrapper.xen610.XenServer610MigrateWithStorageSendCommandWrapper.execute(com.cloud.agent.api.MigrateWithStorageSendCommand,
> > > > com.cloud.hypervisor.xenserver.resource.XenServer610Resource)()
> > > > 70                 // happens when the host/resource on which the
> > command
> > > > has to be executed is owned
> > > > 71                 // by the second management server. The
> > > > serialization/deserialization of the command
> > > > 72                 // and answers fails as the xapi SR and Network
> > class
> > > > type isn't understand by the
> > > > 73                 // agent attache. Seriliaze the SR and Network
> > objects
> > > > here to a string and pass in
> > > > 74                 // the answer object. It'll be deserialzed and
> > object
> > > > created in migrate with
> > > > 75                 // storage send command execution.
> > > > >>>     CID 1349986:  FindBugs: Dodgy code  (FB.DLS_DEAD_LOCAL_STORE)
> > > > >>>     Dead store to gson
> > > > 76                 Gson gson = new Gson();
> > > > 77                 final Map<String, String> other = new
> > HashMap<String,
> > > > String>();
> > > > 78                 other.put("live", "true");
> > > > 79
> > > > 80                 // Create the vdi map which tells what volumes of
> > the
> > > > vm need to go
> > > > 81                 // on which sr on the destination.
> > > >
> > > > ** CID 1349987:  FindBugs: Dodgy code  (FB.DLS_DEAD_LOCAL_STORE)
> > > >
> > >
> >
> /plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageReceiveCommandWrapper.java:
> > > > 70 in
> > > >
> > >
> >
> com.cloud.hypervisor.xenserver.resource.wrapper.xen610.XenServer610MigrateWithStorageReceiveCommandWrapper.execute(com.cloud.agent.api.MigrateWithStorageReceiveCommand,
> > > > com.cloud.hypervisor.xenserver.resource.XenServer610Resource)()
> > > >
> > > >
> > > >
> > > >
> > >
> >
> ________________________________________________________________________________________________________
> > > > *** CID 1349987:  FindBugs: Dodgy code  (FB.DLS_DEAD_LOCAL_STORE)
> > > >
> > >
> >
> /plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageReceiveCommandWrapper.java:
> > > > 70 in
> > > >
> > >
> >
> com.cloud.hypervisor.xenserver.resource.wrapper.xen610.XenServer610MigrateWithStorageReceiveCommandWrapper.execute(com.cloud.agent.api.MigrateWithStorageReceiveCommand,
> > > > com.cloud.hypervisor.xenserver.resource.XenServer610Resource)()
> > > > 64                 // happens when the host/resource on which the
> > command
> > > > has to be executed is owned
> > > > 65                 // by the second management server. The
> > > > serialization/deserialization of the command
> > > > 66                 // and answers fails as the xapi SR and Network
> > class
> > > > type isn't understand by the
> > > > 67                 // agent attache. Seriliaze the SR and Network
> > objects
> > > > here to a string and pass in
> > > > 68                 // the answer object. It'll be deserialzed and
> > object
> > > > created in migrate with
> > > > 69                 // storage send command execution.
> > > > >>>     CID 1349987:  FindBugs: Dodgy code  (FB.DLS_DEAD_LOCAL_STORE)
> > > > >>>     Dead store to gson
> > > > 70                 Gson gson = new Gson();
> > > > 71                 // Get a map of all the SRs to which the vdis will
> > be
> > > > migrated.
> > > > 72                 final List<Pair<VolumeTO, Object>> volumeToSr
=
> new
> > > > ArrayList<Pair<VolumeTO, Object>>();
> > > > 73                 for (final Pair<VolumeTO, StorageFilerTO> entry
:
> > > > volumeToFiler) {
> > > > 74                     final StorageFilerTO storageFiler =
> > > entry.second();
> > > > 75                     final SR sr =
> > > > xenServer610Resource.getStorageRepository(connection,
> > > > storageFiler.getUuid());
> > > >
> > > >
> > > >
> > > >
> > >
> >
> ________________________________________________________________________________________________________
> > > > To view the defects in Coverity Scan visit,
> > > > https://scan.coverity.com/projects/cloudstack?tab=overview
> > > >
> > > > To manage Coverity Scan email notifications for "
> > > dev@cloudstack.apache.org <javascript:;>",
> > > > click
> > > >
> > >
> >
> https://scan.coverity.com/subscriptions/edit?email=dev%40cloudstack.apache.org&token=494aabd5ba647999fa41b6d766646231
> > > >
> > > >
> > >
> > >
> > > --
> > > Daan
> > >
> >
>
>
>
> --
> Daan
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message