cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Koushik Das <koushik....@citrix.com>
Subject RE: Coverity Scan Report: July 11 2014
Date Mon, 14 Jul 2014 04:44:33 GMT
Should commits be reverted if they are not "Findbugs" compliant? Otherwise defect density would
never come down.

-----Original Message-----
From: Santhosh Edukulla [mailto:santhosh.edukulla@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.Connection,
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.Connection,
java.util.List)()
/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1041 in com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connection,
java.util.List)()
/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1109 in com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connection,
java.util.List)()
/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1069 in com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connection,
java.util.List)()
/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1077 in com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connection,
java.util.List)()
/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java: 1109 in com.cloud.upgrade.dao.Upgrade410to420.updateNonLegacyZones(java.sql.Connection,
java.util.List)()

** CID 1225204:  REC: RuntimeException capture  (FB.REC_CATCH_EXCEPTION)
/plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/SecurityGroupHttpClient.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(java.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.Long,
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/datastore/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.Connection,
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.Connection,
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.Connection,
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.Connection,
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.Connection,
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.Connection,
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.Connection,
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/SecurityGroupHttpClient.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(java.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.dropInvalidKeyFromStoragePoolTable(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.Long,
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/datastore/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.SolidFireSharedPrimaryDataStoreLifeCycle$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;

Mime
View raw message