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

[Rodrigo Trujillo]

Reviewed-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>

On 06/07/2016 01:50 AM, Ramon Medeiros wrote:
> 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)))