Fixed Coverity reported performance issues like inefficient string co…

…ncatenations, wrong boxing or unboxing types, inefficent map element retrievals

Signed-off-by: Daan Hoogland <[email protected]>

api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java

@@ -86,8 +86,8 @@ public Map<String, String> getDetails() {
  Iterator iter = parameterCollection.iterator();
  while (iter.hasNext()) {
  HashMap<String, String> value = (HashMap<String, String>)iter.next();
- for (String key : value.keySet()) {
- customparameterMap.put(key, value.get(key));
+ for (Map.Entry<String, String> entry : value.entrySet()) {
+ customparameterMap.put(entry.getKey(), entry.getValue());
  }
  }
  }

api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java

@@ -81,8 +81,8 @@ public Map<String, String> getDetails() {
  Iterator iter = parameterCollection.iterator();
  while (iter.hasNext()) {
  HashMap<String, String> value = (HashMap<String, String>)iter.next();
- for (String key : value.keySet()) {
- customparameterMap.put(key, value.get(key));
+ for (Map.Entry<String,String>entry : value.entrySet()) {
+ customparameterMap.put(entry.getKey(), entry.getValue());
  }
  }
  }

api/src/org/apache/cloudstack/context/CallContext.java

@@ -332,8 +332,8 @@ public Map<Object, Object> getContextParameters() {
 
  public void putContextParameters(Map<Object, Object> details){
  if (details == null) return;
- for(Object key : details.keySet()){
- putContextParameter(key, details.get(key));
+ for(Map.Entry<Object,Object>entry : details.entrySet()){
+ putContextParameter(entry.getKey(), entry.getValue());
  }
  }
 

core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java

@@ -476,15 +476,14 @@ private List<ConfigItem> generateConfig(LoadBalancerConfigCommand cmd) {
  LoadBalancerConfigurator cfgtr = new HAProxyConfigurator();
 
  String[] config = cfgtr.generateConfiguration(cmd);
- String tmpCfgFileContents = "";
+ StringBuffer buff = new StringBuffer();
  for (int i = 0; i < config.length; i++) {
- tmpCfgFileContents += config[i];
- tmpCfgFileContents += "\n";
+ buff.append(config[i]);
+ buff.append("\n");
  }
-
  String tmpCfgFilePath = "/etc/haproxy/";
  String tmpCfgFileName = "haproxy.cfg.new." + String.valueOf(System.currentTimeMillis());
- cfg.add(new ConfigItem(tmpCfgFilePath, tmpCfgFileName, tmpCfgFileContents));
+ cfg.add(new ConfigItem(tmpCfgFilePath, tmpCfgFileName, buff.toString()));
 
  String[][] rules = cfgtr.generateFwRules(cmd);
 
@@ -610,41 +609,58 @@ private List<ConfigItem> generateConfig(DeleteIpAliasCommand cmd) {
  LinkedList<ConfigItem> cfg = new LinkedList<>();
 
  String args = "";
+ StringBuffer buff = new StringBuffer();
  List<IpAliasTO> revokedIpAliasTOs = cmd.getDeleteIpAliasTos();
  for (IpAliasTO ipAliasTO : revokedIpAliasTOs) {
- args = args + ipAliasTO.getAlias_count() + ":" + ipAliasTO.getRouterip() + ":" + ipAliasTO.getNetmask() + "-";
- }
+ buff.append(ipAliasTO.getAlias_count());
+ buff.append(":");
+ buff.append(ipAliasTO.getRouterip());
+ buff.append(":");
+ buff.append(ipAliasTO.getNetmask());
+ buff.append("-");
+ }
  //this is to ensure that thre is some argument passed to the deleteipAlias script when there are no revoked rules.
- args = args + "- ";
+ buff.append("- ");
  List<IpAliasTO> activeIpAliasTOs = cmd.getCreateIpAliasTos();
  for (IpAliasTO ipAliasTO : activeIpAliasTOs) {
- args = args + ipAliasTO.getAlias_count() + ":" + ipAliasTO.getRouterip() + ":" + ipAliasTO.getNetmask() + "-";
- }
-
- cfg.add(new ConfigItem(VRScripts.IPALIAS_DELETE, args));
+ buff.append(ipAliasTO.getAlias_count());
+ buff.append(":");
+ buff.append(ipAliasTO.getRouterip());
+ buff.append(":");
+ buff.append(ipAliasTO.getNetmask());
+ buff.append("-");
+ }
+ cfg.add(new ConfigItem(VRScripts.IPALIAS_DELETE, buff.toString()));
  return cfg;
  }
 
  private List<ConfigItem> generateConfig(DnsMasqConfigCommand cmd) {
  LinkedList<ConfigItem> cfg = new LinkedList<>();
 
  List<DhcpTO> dhcpTos = cmd.getIps();
- String args = "";
+ StringBuffer buff = new StringBuffer();
  for (DhcpTO dhcpTo : dhcpTos) {
- args = args + dhcpTo.getRouterIp() + ":" + dhcpTo.getGateway() + ":" + dhcpTo.getNetmask() + ":" + dhcpTo.getStartIpOfSubnet() + "-";
- }
-
- cfg.add(new ConfigItem(VRScripts.DNSMASQ_CONFIG, args));
+ buff.append(dhcpTo.getRouterIp());
+ buff.append(":");
+ buff.append(dhcpTo.getGateway());
+ buff.append(":");
+ buff.append(dhcpTo.getNetmask());
+ buff.append(":");
+ buff.append(dhcpTo.getStartIpOfSubnet());
+ buff.append("-");
+ }
+ cfg.add(new ConfigItem(VRScripts.DNSMASQ_CONFIG, buff.toString()));
  return cfg;
  }
 
  private CheckS2SVpnConnectionsAnswer execute(CheckS2SVpnConnectionsCommand cmd) {
- String args = "";
+
+ StringBuffer buff = new StringBuffer();
  for (String ip : cmd.getVpnIps()) {
- args += ip + " ";
+ buff.append(ip);
+ buff.append(" ");
  }
-
- ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), VRScripts.S2SVPN_CHECK, args);
+ ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), VRScripts.S2SVPN_CHECK, buff.toString());
  return new CheckS2SVpnConnectionsAnswer(cmd, result.isSuccess(), result.getDetails());
  }
 

engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java

@@ -126,7 +126,7 @@ protected AgentAttache(final AgentManagerImpl agentMgr, final long id, final Str
  _maintenance = maintenance;
  _requests = new LinkedList<Request>();
  _agentMgr = agentMgr;
- _nextSequence = new Long(s_rand.nextInt(Short.MAX_VALUE)) << 48;
+ _nextSequence = new Long(s_rand.nextInt(Short.MAX_VALUE)).longValue() << 48;
  }
 
  public synchronized long getNextSequence() {

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -643,8 +643,8 @@ protected String callHostPluginAsync(Connection conn, String plugin,
  Map<String, String> args = new HashMap<String, String>();
  Task task = null;
  try {
- for (String key : params.keySet()) {
- args.put(key, params.get(key));
+ for (Map.Entry< String, String > entry : params.entrySet()) {
+ args.put(entry.getKey(), entry.getValue());
  }
  if (s_logger.isTraceEnabled()) {
  s_logger.trace("callHostPlugin executing for command " + cmd
@@ -2313,8 +2313,8 @@ protected GetVmStatsAnswer execute(GetVmStatsCommand cmd) {
  return new GetVmStatsAnswer(cmd, vmStatsNameMap);
  }
 
- for (String vmUUID : vmStatsUUIDMap.keySet()) {
- vmStatsNameMap.put(vmNames.get(vmUUIDs.indexOf(vmUUID)), vmStatsUUIDMap.get(vmUUID));
+ for (Map.Entry<String,VmStatsEntry>entry : vmStatsUUIDMap.entrySet()) {
+ vmStatsNameMap.put(vmNames.get(vmUUIDs.indexOf(entry.getKey())), entry.getValue());
  }
 
  return new GetVmStatsAnswer(cmd, vmStatsNameMap);
@@ -2387,8 +2387,8 @@ protected HashMap<String, VmStatsEntry> getVmStats(Connection conn, GetVmStatsCo
 
  }
 
- for (String vmUUID : vmResponseMap.keySet()) {
- VmStatsEntry vmStatsAnswer = vmResponseMap.get(vmUUID);
+ for (Map.Entry<String, VmStatsEntry> entry: vmResponseMap.entrySet()) {
+ VmStatsEntry vmStatsAnswer = entry.getValue();
 
  if (vmStatsAnswer.getNumCPUs() != 0) {
  vmStatsAnswer.setCPUUtilization(vmStatsAnswer.getCPUUtilization() / vmStatsAnswer.getNumCPUs());
@@ -6533,8 +6533,9 @@ private VM createWorkingVM(Connection conn, String vmName, String guestOSType, L
  s_logger.warn("Unable to find vdi by uuid: " + vdiUuid + ", skip it");
  }
  }
- for (VDI vdi : vdiMap.keySet()) {
- VolumeObjectTO volumeTO = vdiMap.get(vdi);
+ for (Map.Entry<VDI, VolumeObjectTO>entry : vdiMap.entrySet()) {
+ VDI vdi = entry.getKey();
+ VolumeObjectTO volumeTO = entry.getValue();
  VBD.Record vbdr = new VBD.Record();
  vbdr.VM = vm;
  vbdr.VDI = vdi;

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerPoolVms.java

@@ -80,8 +80,10 @@ public int size(String clusterId) {
  public String toString() {
  StringBuilder sbuf = new StringBuilder("PoolVms=");
  for (HashMap<String/* vm name */, Pair<String/* host uuid */, State/* vm state */>> clusterVM : _clusterVms.values()) {
- for (String vmname : clusterVM.keySet()) {
- sbuf.append(vmname).append("-").append(clusterVM.get(vmname).second()).append(",");
+ for (Map.Entry<String,Pair<String,State>> entry: clusterVM.entrySet()) {
+ String vmname = entry.getKey();
+ Pair<String,State> vmstate= entry.getValue();
+ sbuf.append(vmname).append("-").append(vmstate.second()).append(",");
  }
  }
  return sbuf.toString();

plugins/network-elements/netscaler/src/com/cloud/network/element/NetscalerElement.java

@@ -677,11 +677,13 @@ public boolean verifyServicesCombination(Set<Service> services) {
  // NetScaler can only act as Lb and Static Nat service provider
  if (services != null && !services.isEmpty() && !netscalerServices.containsAll(services)) {
  s_logger.warn("NetScaler network element can only support LB and Static NAT services and service combination " + services + " is not supported.");
- String servicesList = "";
+
+ StringBuffer buff = new StringBuffer();
  for (Service service : services) {
- servicesList += service.getName() + " ";
+ buff.append(service.getName());
+ buff.append(" ");
  }
- s_logger.warn("NetScaler network element can only support LB and Static NAT services and service combination " + servicesList + " is not supported.");
+ s_logger.warn("NetScaler network element can only support LB and Static NAT services and service combination " + buff.toString() + " is not supported.");
  s_logger.warn("NetScaler network element can only support LB and Static NAT services and service combination " + services + " is not supported.");
  return false;
  }

plugins/network-elements/netscaler/src/com/cloud/network/resource/NetscalerResource.java

@@ -1763,14 +1763,14 @@ private static String generateGslbServerName(String serverIP) {
  }
 
  private static String genGslbObjectName(Object... args) {
- String objectName = "";
+ StringBuffer buff = new StringBuffer();
  for (int i = 0; i < args.length; i++) {
- objectName += args[i];
+ buff.append(args[i]);
  if (i != args.length - 1) {
- objectName += "-";
+ buff.append("-");
  }
  }
- return objectName;
+ return buff.toString();
  }
  }
 
@@ -3767,14 +3767,14 @@ private String generateSslCertKeyName(String fingerPrint) {
  }
 
  private String genObjectName(Object... args) {
- String objectName = "";
+ StringBuffer buff = new StringBuffer();
  for (int i = 0; i < args.length; i++) {
- objectName += args[i];
+ buff.append(args[i]);
  if (i != args.length - 1) {
- objectName += _objectNamePathSep;
+ buff.append(_objectNamePathSep);
  }
  }
- return objectName;
+ return buff.toString();
  }
 
  @Override

plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LDAPConfigCmd.java

@@ -155,7 +155,7 @@ public void setHostname(String hostname) {
  }
 
  public Integer getPort() {
- return port <= 0 ? 389 : port;
+ return (Integer)(port.intValue() <= 0 ? 389 : port.intValue());
  }
 
  public void setPort(Integer port) {

server/src/com/cloud/api/ApiResponseHelper.java

@@ -1906,7 +1906,8 @@ public NetworkResponse createNetworkResponse(ResponseView view, Network network)
  Map<Service, Map<Capability, String>> serviceCapabilitiesMap = ApiDBUtils.getNetworkCapabilities(network.getId(), network.getDataCenterId());
  List<ServiceResponse> serviceResponses = new ArrayList<ServiceResponse>();
  if (serviceCapabilitiesMap != null) {
- for (Service service : serviceCapabilitiesMap.keySet()) {
+ for (Map.Entry<Service, Map<Capability, String>>entry : serviceCapabilitiesMap.entrySet()) {
+ Service service = entry.getKey();
  ServiceResponse serviceResponse = new ServiceResponse();
  // skip gateway service
  if (service == Service.Gateway) {
@@ -1916,11 +1917,12 @@ public NetworkResponse createNetworkResponse(ResponseView view, Network network)
 
  // set list of capabilities for the service
  List<CapabilityResponse> capabilityResponses = new ArrayList<CapabilityResponse>();
- Map<Capability, String> serviceCapabilities = serviceCapabilitiesMap.get(service);
+ Map<Capability, String> serviceCapabilities = entry.getValue();
  if (serviceCapabilities != null) {
- for (Capability capability : serviceCapabilities.keySet()) {
+ for (Map.Entry<Capability,String> ser_cap_entries : serviceCapabilities.entrySet()) {
+ Capability capability = ser_cap_entries.getKey();
  CapabilityResponse capabilityResponse = new CapabilityResponse();
- String capabilityValue = serviceCapabilities.get(capability);
+ String capabilityValue = ser_cap_entries.getValue();
  capabilityResponse.setName(capability.getName());
  capabilityResponse.setValue(capabilityValue);
  capabilityResponse.setObjectName("capability");
@@ -2605,15 +2607,17 @@ public VpcResponse createVpcResponse(ResponseView view, Vpc vpc) {
 
  Map<Service, Set<Provider>> serviceProviderMap = ApiDBUtils.listVpcOffServices(vpc.getVpcOfferingId());
  List<ServiceResponse> serviceResponses = new ArrayList<ServiceResponse>();
- for (Service service : serviceProviderMap.keySet()) {
+ for (Map.Entry<Service,Set<Provider>>entry : serviceProviderMap.entrySet()) {
+ Service service = entry.getKey();
+ Set<Provider> serviceProviders = entry.getValue();
  ServiceResponse svcRsp = new ServiceResponse();
  // skip gateway service
  if (service == Service.Gateway) {
  continue;
  }
  svcRsp.setName(service.getName());
  List<ProviderResponse> providers = new ArrayList<ProviderResponse>();
- for (Provider provider : serviceProviderMap.get(service)) {
+ for (Provider provider : serviceProviders) {
  if (provider != null) {
  ProviderResponse providerRsp = new ProviderResponse();
  providerRsp.setName(provider.getName());

server/src/com/cloud/api/ApiServer.java

@@ -478,7 +478,7 @@ public String handleRequest(final Map params, final String responseType, final S
  s_logger.error("invalid request, no command sent");
  if (s_logger.isTraceEnabled()) {
  s_logger.trace("dumping request parameters");
- for (final Object key : params.keySet()) {
+ for (final  Object key : params.keySet()) {
  final String keyStr = (String)key;
  final String[] value = (String[])params.get(key);
  s_logger.trace(" key: " + keyStr + ", value: " + ((value == null) ? "'null'" : value[0]));
@@ -652,7 +652,7 @@ private String queueCommand(final BaseCmd cmdObj, final Map<String, String> para
 
  // save the scheduled event
  final Long eventId =
- ActionEventUtils.onScheduledActionEvent((callerUserId == null) ? User.UID_SYSTEM : callerUserId, asyncCmd.getEntityOwnerId(), asyncCmd.getEventType(),
+ ActionEventUtils.onScheduledActionEvent((callerUserId == null) ? (Long)User.UID_SYSTEM : callerUserId, asyncCmd.getEntityOwnerId(), asyncCmd.getEventType(),
  asyncCmd.getEventDescription(), asyncCmd.isDisplay(), startEventId);
  if (startEventId == 0) {
  // There was no create event before, set current event id as start eventId

server/src/com/cloud/api/dispatch/ParamProcessWorker.java

@@ -33,6 +33,7 @@
 
 import javax.inject.Inject;
 
+
 import org.apache.log4j.Logger;
 
 import org.apache.cloudstack.acl.ControlledEntity;
@@ -229,9 +230,10 @@ private void doAccessChecks(BaseCmd cmd, Map<Object, AccessType> entitiesToAcces
  if (!entitiesToAccess.isEmpty()) {
  // check that caller can access the owner account.
  _accountMgr.checkAccess(caller, null, true, owner);
- for (Object entity : entitiesToAccess.keySet()) {
+ for (Map.Entry<Object,AccessType>entry : entitiesToAccess.entrySet()) {
+ Object entity = entry.getKey();
  if (entity instanceof ControlledEntity) {
- _accountMgr.checkAccess(caller, entitiesToAccess.get(entity), true, (ControlledEntity) entity);
+ _accountMgr.checkAccess(caller, entry.getValue(), true, (ControlledEntity) entity);
  } else if (entity instanceof InfrastructureEntity) {
  // FIXME: Move this code in adapter, remove code from
  // Account manager

server/src/com/cloud/api/dispatch/ParamUnpackWorker.java

@@ -33,7 +33,8 @@ public class ParamUnpackWorker implements DispatchWorker {
  public void handle(final DispatchTask task) throws ServerApiException {
  final Map<String, Object> lowercaseParams = new HashMap<String, Object>();
  final Map<String, String> params = task.getParams();
- for (final String key : params.keySet()) {
+ for (final Map.Entry<String,String> entry : params.entrySet()) {
+ final String key = entry.getKey();
  final int arrayStartIndex = key.indexOf('[');
  final int arrayStartLastIndex = key.lastIndexOf('[');
  if (arrayStartIndex != arrayStartLastIndex) {
@@ -99,11 +100,11 @@ public void handle(final DispatchTask task) throws ServerApiException {
  }
 
  // we are ready to store the value for a particular field into the map for this object
- mapValue.put(fieldName, params.get(key));
+ mapValue.put(fieldName, entry.getValue());
 
  lowercaseParams.put(paramName, mapArray);
  } else {
- lowercaseParams.put(key.toLowerCase(), params.get(key));
+ lowercaseParams.put(key.toLowerCase(), entry.getValue());
  }
  }