[Kimchi-devel] [PATCH][Kimchi] Do not use default value when declare a function

[Ramon Medeiros]

For mutable object as a default parameter in function and
method-declarations the problem is, that the evaluation and creation
takes place at exactly the same moment. The python-parser reads the
function-head and evaluates it at the same moment.

Most beginers assume that a new object is created at every call, but
that's not correct. One object (in your example a list) is created at
the moment of declaration and not on demand when you are calling the
method.

For imutable objects that's not a problem, because even if all calls
share the same object, it's imutable and therefore it's properties
remain the same.

As a convention, must be used the None object for defaults to indicate the use
of a default initialization, which now can take place in the
function-body, which naturally is evaluated at call-time.

Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
---
 mockmodel.py          | 5 ++++-
 model/networks.py     | 5 ++++-
 model/templates.py    | 5 ++++-
 model/vmsnapshots.py  | 4 +++-
 network.py            | 5 ++++-
 xmlutils/bootorder.py | 5 ++++-
 xmlutils/cpu.py       | 4 +++-
 7 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/mockmodel.py b/mockmodel.py
index 6c70472..9f9de52 100644
--- a/mockmodel.py
+++ b/mockmodel.py
@@ -714,7 +714,10 @@ class MockDevices(object):
 
 
 class MockVMSnapshot(object):
-    def __init__(self, name, params={}):
+    def __init__(self, name, params=None):
+        if params is None:
+            params = {}
+
         self.name = name
         self.current = True
 
diff --git a/model/networks.py b/model/networks.py
index 3b2a9e2..4b722d3 100644
--- a/model/networks.py
+++ b/model/networks.py
@@ -123,7 +123,10 @@ class NetworksModel(object):
         names = conn.listNetworks() + conn.listDefinedNetworks()
         return sorted(map(lambda x: x.decode('utf-8'), names))
 
-    def _get_available_address(self, addr_pools=[]):
+    def _get_available_address(self, addr_pools=None):
+        if addr_pools is None:
+            addr_pools = []
+
         invalid_addrs = []
         for net_name in self.get_list():
             network = NetworkModel.get_network(self.conn.get(), net_name)
diff --git a/model/templates.py b/model/templates.py
index 25bc017..a299c85 100644
--- a/model/templates.py
+++ b/model/templates.py
@@ -177,7 +177,10 @@ class TemplateModel(object):
         self.templates = TemplatesModel(**kargs)
 
     @staticmethod
-    def get_template(name, objstore, conn, overrides={}):
+    def get_template(name, objstore, conn, overrides=None):
+        if overrides is None:
+            overrides = {}
+
         with objstore as session:
             params = session.get('template', name)
         if overrides and 'storagepool' in overrides:
diff --git a/model/vmsnapshots.py b/model/vmsnapshots.py
index fd17d2f..6f2483c 100644
--- a/model/vmsnapshots.py
+++ b/model/vmsnapshots.py
@@ -40,7 +40,7 @@ class VMSnapshotsModel(object):
         self.vmstorages = VMStoragesModel(**kargs)
         self.vmstorage = VMStorageModel(**kargs)
 
-    def create(self, vm_name, params={}):
+    def create(self, vm_name, params=None):
         """Create a snapshot with the current domain state.
 
         The VM must be stopped and contain only disks with format 'qcow2';
@@ -55,6 +55,8 @@ class VMSnapshotsModel(object):
         Return:
         A Task running the operation.
         """
+        if params is None:
+            params = {}
         vir_dom = VMModel.get_vm(vm_name, self.conn)
         if DOM_STATE_MAP[vir_dom.info()[0]] != u'shutoff':
             raise InvalidOperation('KCHSNAP0001E', {'vm': vm_name})
diff --git a/network.py b/network.py
index dd7b50f..ec567ca 100644
--- a/network.py
+++ b/network.py
@@ -52,7 +52,10 @@ def get_dev_netaddrs():
 
 # used_nets should include all the subnet allocated in libvirt network
 # will get host network by get_dev_netaddrs
-def get_one_free_network(used_nets, nets_pool=PrivateNets):
+def get_one_free_network(used_nets, nets_pool=None):
+    if nets_pool is None:
+        nets_pool = PrivateNets
+
     def _get_free_network(nets, used_nets):
         for net in nets.subnet(new_prefix=24):
             if not any(net.overlaps(used) for used in used_nets):
diff --git a/xmlutils/bootorder.py b/xmlutils/bootorder.py
index 28457e7..9e444bd 100644
--- a/xmlutils/bootorder.py
+++ b/xmlutils/bootorder.py
@@ -21,7 +21,7 @@ import lxml.etree as ET
 from lxml.builder import E
 
 
-def get_bootorder_xml(boot_order=['hd', 'cdrom', 'network']):
+def get_bootorder_xml(boot_order=None):
     """
     Returns the XML for boot order. The default return includes the following:
 
@@ -31,6 +31,9 @@ def get_bootorder_xml(boot_order=['hd', 'cdrom', 'network']):
 
     To a different boot order, specify the order by a list as argument.
     """
+    if boot_order is None:
+        boot_order = ['hd', 'cdrom', 'network']
+
     boot_xml = ''
     for device in boot_order:
         boot = E.boot(dev=device)
diff --git a/xmlutils/cpu.py b/xmlutils/cpu.py
index 08a6afb..7548be2 100644
--- a/xmlutils/cpu.py
+++ b/xmlutils/cpu.py
@@ -45,7 +45,7 @@ def get_topology_xml(cpu_topo):
     return ET.tostring(xml)
 
 
-def get_cpu_xml(cpus, memory, cpu_topo={}):
+def get_cpu_xml(cpus, memory, cpu_topo=None):
     # Returns the libvirt CPU element based on given numa and topology
     # CPU element will always have numa element
     #   <cpu>
@@ -54,6 +54,8 @@ def get_cpu_xml(cpus, memory, cpu_topo={}):
     #      </numa>
     #      <topology sockets='1' cores='2' threads='1'/>
     #   </cpu>
+    if cpu_topo is None:
+        cpu_topo = {}
     xml = E.cpu(ET.fromstring(get_numa_xml(cpus, memory)))
     if cpu_topo:
         xml.insert(0, ET.fromstring(get_topology_xml(cpu_topo)))
-- 
2.5.5