On Tue, Dec 2, 2014 at 4:50 PM, Rohit Yadav <rohityada...@gmail.com> wrote: > Hi Daan/Wilder, > > Quick question about fixing the following coverity issue (in-line); > > On Tue, Dec 2, 2014 at 8:45 PM, <d...@apache.org> wrote: > >> Repository: cloudstack >> Updated Branches: >> refs/heads/master 818957de0 -> 6dd30eaf1 >> >> >> CID-1256273/CID-1256274/CID-1256275 leaky resources plus switch >> statement warning >> >> reviewed by Wilder Rodrigues >> >> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo >> Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/6dd30eaf >> Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/6dd30eaf >> Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/6dd30eaf >> >> Branch: refs/heads/master >> Commit: 6dd30eaf14f323cd84252647c490e84982b0a853 >> Parents: 818957d >> Author: Daan Hoogland <dhoogl...@schubergphilis.com> >> Authored: Tue Dec 2 16:14:34 2014 +0100 >> Committer: Daan Hoogland <d...@onecht.net> >> Committed: Tue Dec 2 16:14:34 2014 +0100 >> >> ---------------------------------------------------------------------- >> .../com/cloud/upgrade/dao/Upgrade442to450.java | 28 ++++++-------------- >> 1 file changed, 8 insertions(+), 20 deletions(-) >> ---------------------------------------------------------------------- >> >> >> >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6dd30eaf/engine/schema/src/com/cloud/upgrade/dao/Upgrade442to450.java >> ---------------------------------------------------------------------- >> diff --git a/engine/schema/src/com/cloud/upgrade/dao/Upgrade442to450.java >> b/engine/schema/src/com/cloud/upgrade/dao/Upgrade442to450.java >> index aeb44a1..9fe1319 100644 >> --- a/engine/schema/src/com/cloud/upgrade/dao/Upgrade442to450.java >> +++ b/engine/schema/src/com/cloud/upgrade/dao/Upgrade442to450.java >> @@ -115,9 +115,6 @@ public class Upgrade442to450 implements DbUpgrade { >> } >> >> private void upgradeMemoryOfInternalLoadBalancervmOffering(Connection >> conn) { >> - PreparedStatement updatePstmt = null; >> - PreparedStatement selectPstmt = null; >> - ResultSet selectResultSet = null; >> int newRamSize = 256; //256MB >> long serviceOfferingId = 0; >> >> @@ -126,10 +123,9 @@ public class Upgrade442to450 implements DbUpgrade { >> * We should not update/modify any user-defined offering. >> */ >> >> - try { >> - selectPstmt = conn.prepareStatement("SELECT id FROM >> `cloud`.`service_offering` WHERE vm_type='internalloadbalancervm'"); >> - updatePstmt = conn.prepareStatement("UPDATE >> `cloud`.`service_offering` SET ram_size=? WHERE id=?"); >> - selectResultSet = selectPstmt.executeQuery(); >> + try (PreparedStatement selectPstmt = >> conn.prepareStatement("SELECT id FROM `cloud`.`service_offering` WHERE >> vm_type='internalloadbalancervm'"); >> + PreparedStatement updatePstmt = >> conn.prepareStatement("UPDATE `cloud`.`service_offering` SET ram_size=? >> WHERE id=?"); >> + ResultSet selectResultSet = selectPstmt.executeQuery()){ >> if(selectResultSet.next()) { >> serviceOfferingId = selectResultSet.getLong("id"); >> } >> @@ -139,19 +135,6 @@ public class Upgrade442to450 implements DbUpgrade { >> updatePstmt.executeUpdate(); >> } catch (SQLException e) { >> throw new CloudRuntimeException("Unable to upgrade ram_size >> of service offering for internal loadbalancer vm. ", e); >> - } finally { >> - try { >> - if (selectPstmt != null) { >> - selectPstmt.close(); >> - } >> - if (selectResultSet != null) { >> - selectResultSet.close(); >> - } >> - if (updatePstmt != null) { >> - updatePstmt.close(); >> - } >> - } catch (SQLException e) { >> - } >> > > Why are we removing closing statements and resultsets here?
because of the try-with-resource (automatic closing of resources) > > >> } >> s_logger.debug("Done upgrading RAM for service offering of >> internal loadbalancer vm to " + newRamSize); >> } >> @@ -188,6 +171,8 @@ public class Upgrade442to450 implements DbUpgrade { >> break; >> case LXC: >> hypervisorsListInUse.add(Hypervisor.HypervisorType.LXC); >> break; >> + default: // no action on cases Any, BareMetal, >> None, Ovm, Parralels, Simulator and VirtualBox: >> + break; >> } >> } >> } catch (SQLException e) { >> @@ -258,6 +243,8 @@ public class Upgrade442to450 implements DbUpgrade { >> pstmt.executeUpdate(); >> pstmt.close(); >> } else { >> + rs.close(); >> + pstmt.close(); >> if >> (hypervisorsListInUse.contains(hypervisorAndTemplateName.getKey())){ >> throw new CloudRuntimeException("4.5.0 " + >> hypervisorAndTemplateName.getKey() + " SystemVm template not found. Cannot >> upgrade system Vms"); >> } else { >> @@ -286,6 +273,7 @@ public class Upgrade442to450 implements DbUpgrade { >> pstmt.close(); >> > > Why are we keeping statements and resultsets her? the refactoring of all those usages of the statement would require major refactorring for this method. I choose not to do this for this one. could be doen as well. > > Regards. -- Daan