+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;
