> On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote: > > engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java, line 310 > > <https://reviews.apache.org/r/23226/diff/1/?file=622487#file622487line310> > > > > Shouldn't this be inside the try-with-resources bit of the try keyword?
Here we are taking care of closing the resource with "prepareAutoCloseStatement", compared to normal call of "prepareStatement" i believe. We used try with resource where prepare through auto close was not followed. Going ahead, we can change it to try with resource, to make it uniform, but here other leak issue is fixed i believe. > On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote: > > engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java, > > line 71 > > <https://reviews.apache.org/r/23226/diff/1/?file=622500#file622500line71> > > > > To make it more clear for the readers of the code why not write 'if > > (params == null) { throw blabla }' > > > > We can, As such possibility of null referencing is fixed, going ahead, i can add a log and throw accordingly, but on second note, i assume that the calling function based upon the callable output can log and throw accordingly there, rather we take care in the called function. Let me know your thoughts. - Santhosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23226/#review47181 ----------------------------------------------------------- On July 2, 2014, 8:44 a.m., Santhosh Edukulla wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23226/ > ----------------------------------------------------------- > > (Updated July 2, 2014, 8:44 a.m.) > > > Review request for cloudstack, Abhinandan Prateek and daan Hoogland. > > > Bugs: coverity > https://issues.apache.org/jira/browse/coverity > > > Repository: cloudstack-git > > > Description > ------- > > Fixed resource leaks, null dererence issues, performance issues and bugs > reported by coverity. Simple bug in generic dao was found during these > changes, fixed that as well. > > > Diffs > ----- > > engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e > engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74 > engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d > engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8 > engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java > c868ac0 > engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java > f27fd60 > engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java > 803288a > engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java > 0fc2ce0 > engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f > engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd > engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c > > engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java > 92793f1 > > engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java > b4ef595 > > engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java > 6b12975 > > framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java > 3d0c3f5 > framework/db/src/com/cloud/utils/db/DbUtil.java 792573c > framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java > e684b8d > > plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java > 579cc24 > server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e > server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e > server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d > > Diff: https://reviews.apache.org/r/23226/diff/ > > > Testing > ------- > > Built the management server, ran the simulator and deployed dc. > > > Thanks, > > Santhosh Edukulla > >
