Or fixed. Feel free to help fellow committers learn by fixing bugs and showing why a certain way of doing things could cause problems later on.
Cheers, Hugo On 14 jul. 2014, at 07:16, Abhinandan Prateek <abhinandan.prat...@citrix.com> wrote: > +1 Any commit that is not findbug compliant should be reverted. > > On 14/07/14 10:14 am, "Koushik Das" <koushik....@citrix.com> wrote: > >> Should commits be reverted if they are not "Findbugs" compliant? >> Otherwise defect density would never come down. >> >> -----Original Message----- >> From: Santhosh Edukulla [mailto:santhosh.eduku...@citrix.com] >> Sent: Friday, 11 July 2014 8:59 PM >> To: dev@cloudstack.apache.org >> Subject: Coverity Scan Report: July 11 2014 >> >> Hi All, >> >> FYI, >> Current Defect Density :3.58 >> Newly Detected: 7, below are new defects reported by coverity scan >> service. >> >> A small note, we should not allow adding new defects to the existing >> reported count, otherwise, current technical debt will never come down. >> Few of these issues can be equally, found by running findbugs or other >> static analysis tools, using IDE locally. Some of these, reported below >> could be fp as well. >> >> How to use findbugs @link below: >> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Using+FindBugs >> >> I believe review submissions, can be made little stringent, if we can >> enforce\atleast inquire for findbugs reports from users for submitted >> reviews. >> >> Regards >> Santhosh >> >> ============================================================ >> Defect(s) Reported-by: Coverity Scan >> Showing 7 of 7 defect(s) >> >> >> ** CID 1225198: Explicit null dereferenced (FORWARD_NULL) >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1055 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> >> ** CID 1225199: Resource leak (RESOURCE_LEAK) >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1031 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1041 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1109 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1069 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1077 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1109 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> >> ** CID 1225204: REC: RuntimeException capture (FB.REC_CATCH_EXCEPTION) >> /plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/Secu >> rityGroupHttpClient.java: 144 in >> com.cloud.baremetal.networkservice.SecurityGroupHttpClient.echo(java.lang. >> String, long, long)() >> >> ** CID 1225203: WMI: Inefficient Map Iterator >> (FB.WMI_WRONG_MAP_ITERATOR) >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade440to450.java: 84 in >> com.cloud.upgrade.dao.Upgrade440to450.dropInvalidKeyFromStoragePoolTable(j >> ava.sql.Connection)() >> >> ** CID 1225202: REC: RuntimeException capture (FB.REC_CATCH_EXCEPTION) >> /engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java: >> 145 in >> com.cloud.usage.dao.UsageSecurityGroupDaoImpl.getUsageRecords(java.lang.Lo >> ng, java.lang.Long, java.util.Date, java.util.Date, boolean, int)() >> >> ** CID 1225201: REC: RuntimeException capture (FB.REC_CATCH_EXCEPTION) >> /engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java: 211 in >> com.cloud.usage.dao.UsageStorageDaoImpl.getUsageRecords(java.lang.Long, >> java.lang.Long, java.util.Date, java.util.Date, boolean, int)() >> >> ** CID 1225200: SIC: Inner class could be made static >> (FB.SIC_INNER_SHOULD_BE_STATIC) >> /plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datast >> ore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java: 296 in () >> >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225198: Explicit null dereferenced (FORWARD_NULL) >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1055 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1049 } else if (key.equalsIgnoreCase("password")) >> { >> 1050 password = value; >> 1051 } else if (key.equalsIgnoreCase("url")) { >> 1052 url = value; >> 1053 } >> 1054 } >>>>> CID 1225198: Explicit null dereferenced (FORWARD_NULL) >>>>> Calling a method on null object "url". >> 1055 String[] tokens = url.split("/"); // url format >> - http://vcenter/dc/cluster >> 1056 String vc = tokens[2]; >> 1057 String dcName = tokens[3]; >> 1058 String guid = dcName + "@" + vc; >> 1059 >> 1060 pstmt = conn.prepareStatement("INSERT INTO >> `cloud`.`vmware_data_center` (uuid, name, guid, vcenter_host, username, >> password) values(?, ?, ?, ?, ?, ?)"); >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225199: Resource leak (RESOURCE_LEAK) >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1031 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1025 ResultSet dcInfo = null; >> 1026 try { >> 1027 for (Long zoneId : zones) { >> 1028 s_logger.debug("Discovered non-legacy zone " + >> zoneId + ". Processing the zone to associate with VMware datacenter."); >> 1029 >> 1030 // All clusters in a non legacy zone will belong >> to the same VMware DC, hence pick the first cluster >>>>> CID 1225199: Resource leak (RESOURCE_LEAK) >>>>> Overwriting "clustersQuery" in "clustersQuery = >>>>> conn.prepareStatement("select id from `cloud`.`cluster` where removed >>>>> is NULL AND data_center_id=?")" leaks the resource that >>>>> "clustersQuery" refers to. >> 1031 clustersQuery = conn.prepareStatement("select id >> from `cloud`.`cluster` where removed is NULL AND data_center_id=?"); >> 1032 clustersQuery.setLong(1, zoneId); >> 1033 clusters = clustersQuery.executeQuery(); >> 1034 clusters.next(); >> 1035 Long clusterId = clusters.getLong("id"); >> 1036 >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1041 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1035 Long clusterId = clusters.getLong("id"); >> 1036 >> 1037 // Get VMware datacenter details from >> cluster_details table >> 1038 String user = null; >> 1039 String password = null; >> 1040 String url = null; >>>>> CID 1225199: Resource leak (RESOURCE_LEAK) >>>>> Overwriting "clusterDetailsQuery" in "clusterDetailsQuery = >>>>> conn.prepareStatement("select name, value from >>>>> `cloud`.`cluster_details` where cluster_id=?")" leaks the resource >>>>> that "clusterDetailsQuery" refers to. >> 1041 clusterDetailsQuery = >> conn.prepareStatement("select name, value from `cloud`.`cluster_details` >> where cluster_id=?"); >> 1042 clusterDetailsQuery.setLong(1, clusterId); >> 1043 clusterDetails = >> clusterDetailsQuery.executeQuery(); >> 1044 while (clusterDetails.next()) { >> 1045 String key = clusterDetails.getString(1); >> 1046 String value = clusterDetails.getString(2); >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1109 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1103 if (pstmt != null) { >> 1104 pstmt.close(); >> 1105 } >> 1106 } catch (SQLException e) { >> 1107 } >> 1108 } >>>>> CID 1225199: Resource leak (RESOURCE_LEAK) >>>>> Variable "clusterDetailsQuery" going out of scope leaks the >>>>> resource it refers to. >> 1109 } >> 1110 >> 1111 private void createPlaceHolderNics(Connection conn) { >> 1112 PreparedStatement pstmt = null; >> 1113 ResultSet rs = null; >> 1114 >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1069 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1063 pstmt.setString(3, guid); >> 1064 pstmt.setString(4, vc); >> 1065 pstmt.setString(5, user); >> 1066 pstmt.setString(6, password); >> 1067 pstmt.executeUpdate(); >> 1068 >>>>> CID 1225199: Resource leak (RESOURCE_LEAK) >>>>> Overwriting "pstmt" in "pstmt = conn.prepareStatement("SELECT id >>>>> FROM `cloud`.`vmware_data_center` where guid=?")" leaks the resource >>>>> that "pstmt" refers to. >> 1069 pstmt = conn.prepareStatement("SELECT id FROM >> `cloud`.`vmware_data_center` where guid=?"); >> 1070 pstmt.setString(1, guid); >> 1071 dcInfo = pstmt.executeQuery(); >> 1072 Long vmwareDcId = -1L; >> 1073 if (dcInfo.next()) { >> 1074 vmwareDcId = dcInfo.getLong("id"); >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1077 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1071 dcInfo = pstmt.executeQuery(); >> 1072 Long vmwareDcId = -1L; >> 1073 if (dcInfo.next()) { >> 1074 vmwareDcId = dcInfo.getLong("id"); >> 1075 } >> 1076 >>>>> CID 1225199: Resource leak (RESOURCE_LEAK) >>>>> Overwriting "pstmt" in "pstmt = conn.prepareStatement("INSERT >>>>> INTO `cloud`.`vmware_data_center_zone_map` (zone_id, >>>>> vmware_data_center_id) values(?, ?)")" leaks the resource that "pstmt" >>>>> refers to. >> 1077 pstmt = conn.prepareStatement("INSERT INTO >> `cloud`.`vmware_data_center_zone_map` (zone_id, vmware_data_center_id) >> values(?, ?)"); >> 1078 pstmt.setLong(1, zoneId); >> 1079 pstmt.setLong(2, vmwareDcId); >> 1080 pstmt.executeUpdate(); >> 1081 } >> 1082 } catch (SQLException e) { >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1109 in >> com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connec >> tion, java.util.List)() >> 1103 if (pstmt != null) { >> 1104 pstmt.close(); >> 1105 } >> 1106 } catch (SQLException e) { >> 1107 } >> 1108 } >>>>> CID 1225199: Resource leak (RESOURCE_LEAK) >>>>> Variable "pstmt" going out of scope leaks the resource it refers >>>>> to. >> 1109 } >> 1110 >> 1111 private void createPlaceHolderNics(Connection conn) { >> 1112 PreparedStatement pstmt = null; >> 1113 ResultSet rs = null; >> 1114 >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225204: REC: RuntimeException capture (FB.REC_CATCH_EXCEPTION) >> /plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/Secu >> rityGroupHttpClient.java: 144 in >> com.cloud.baremetal.networkservice.SecurityGroupHttpClient.echo(java.lang. >> String, long, long)() >> 138 if (httpClient.executeMethod(post) != 200) { >> 139 logger.debug(String.format("echoing baremetal >> security group agent on %s got error: %s", agentIp, >> post.getResponseBodyAsString())); >> 140 } else { >> 141 ret = true; >> 142 } >> 143 break; >>>>> CID 1225204: REC: RuntimeException capture >>>>> (FB.REC_CATCH_EXCEPTION) >>>>> Catching RuntimeExceptions, perhaps unintentionally, with a catch >>>>> block for Exception >> 144 } catch (Exception e) { >> 145 if (count*m >= l) { >> 146 logger.debug(String.format("ping security >> group agent on vm[%s] timeout after %s minutes, starting vm failed, >> count=%s", agentIp, TimeUnit.MILLISECONDS.toSeconds(l), count)); >> 147 break; >> 148 } else { >> 149 logger.debug(String.format("Having pinged >> security group agent on vm[%s] %s times, continue to wait...", agentIp, >> count)); >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225203: WMI: Inefficient Map Iterator >> (FB.WMI_WRONG_MAP_ITERATOR) >> /engine/schema/src/com/cloud/upgrade/dao/Upgrade440to450.java: 84 in >> com.cloud.upgrade.dao.Upgrade440to450.dropInvalidKeyFromStoragePoolTable(j >> ava.sql.Connection)() >> 78 >> 79 keys.add("id_2"); >> 80 uniqueKeys.put("storage_pool", keys); >> 81 >> 82 s_logger.debug("Droping id_2 key from storage_pool table"); >> 83 for (String tableName : uniqueKeys.keySet()) { >>>>> CID 1225203: WMI: Inefficient Map Iterator >>>>> (FB.WMI_WRONG_MAP_ITERATOR) >>>>> >>>>> com.cloud.upgrade.dao.Upgrade440to450.dropInvalidKeyFromStoragePoolTabl >>>>> e(Connection) makes inefficient use of keySet iterator instead of >>>>> entrySet iterator >> 84 DbUpgradeUtils.dropKeysIfExist(conn, tableName, >> uniqueKeys.get(tableName), false); >> 85 } >> 86 } >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225202: REC: RuntimeException capture (FB.REC_CATCH_EXCEPTION) >> /engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java: >> 145 in >> com.cloud.usage.dao.UsageSecurityGroupDaoImpl.getUsageRecords(java.lang.Lo >> ng, java.lang.Long, java.util.Date, java.util.Date, boolean, int)() >> 139 } >> 140 usageRecords.add(new >> UsageSecurityGroupVO(zoneId, acctId, dId, vmId, sgId, createdDate, >> deletedDate)); >> 141 } >> 142 }catch (SQLException e) { >> 143 throw new CloudException("Error getting usage >> records"+e.getMessage(), e); >> 144 } >>>>> CID 1225202: REC: RuntimeException capture >>>>> (FB.REC_CATCH_EXCEPTION) >>>>> Catching RuntimeExceptions, perhaps unintentionally, with a catch >>>>> block for Exception >> 145 } catch (Exception e) { >> 146 txn.rollback(); >> 147 s_logger.warn("Error getting usage >> records:"+e.getMessage(), e); >> 148 } finally { >> 149 txn.close(); >> 150 } >> 151 return usageRecords; >> 152 } >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225201: REC: RuntimeException capture (FB.REC_CATCH_EXCEPTION) >> /engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java: 211 in >> com.cloud.usage.dao.UsageStorageDaoImpl.getUsageRecords(java.lang.Long, >> java.lang.Long, java.util.Date, java.util.Date, boolean, int)() >> 205 usageRecords.add(new UsageStorageVO(id, >> zoneId, acctId, dId, type, sourceId, size, virtualSize, createdDate, >> deletedDate)); >> 206 } >> 207 }catch(SQLException e) >> 208 { >> 209 throw new >> CloudException("getUsageRecords:"+e.getMessage(),e); >> 210 } >>>>> CID 1225201: REC: RuntimeException capture >>>>> (FB.REC_CATCH_EXCEPTION) >>>>> Catching RuntimeExceptions, perhaps unintentionally, with a catch >>>>> block for Exception >> 211 }catch (Exception e) { >> 212 txn.rollback(); >> 213 >> s_logger.error("getUsageRecords:Exception:"+e.getMessage(), e); >> 214 } finally { >> 215 txn.close(); >> 216 } >> 217 return usageRecords; >> 218 } >> >> __________________________________________________________________________ >> ______________________________ >> *** CID 1225200: SIC: Inner class could be made static >> (FB.SIC_INNER_SHOULD_BE_STATIC) >> /plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datast >> ore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java: 296 in () >> 290 return StoragePoolType.VMFS; >> 291 } >> 292 >> 293 throw new CloudRuntimeException("The 'hypervisor' >> parameter must be '" + HypervisorType.XenServer + "' or '" + >> HypervisorType.VMware + "'."); >> 294 } >> 295 >>>>> CID 1225200: SIC: Inner class could be made static >>>>> (FB.SIC_INNER_SHOULD_BE_STATIC) >>>>> Should >>>>> org.apache.cloudstack.storage.datastore.lifecycle.SolidFireSharedPrimar >>>>> yDataStoreLifeCycle$SolidFireCreateVolume be a _static_ inner class? >> 296 private class SolidFireCreateVolume { >> 297 private final SolidFireUtil.SolidFireVolume _sfVolume; >> 298 private final SolidFireUtil.SolidFireAccount _sfAccount; >> 299 >> 300 public >> SolidFireCreateVolume(SolidFireUtil.SolidFireVolume sfVolume, >> SolidFireUtil.SolidFireAccount sfAccount) { >> 301 _sfVolume = sfVolume; >
