From bf9ed50d510d44137c1da6e5c93cdddcc917a965 Mon Sep 17 00:00:00 2001 From: Alexander Todorov Date: Thu, 12 Oct 2017 17:24:18 +0300 Subject: [PATCH] Fix dangerous-default value warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit when default value is list or dict the default arguments are instantiated as objects at the time of definition. This is significant (exposing visible semantics) when the object is mutable. There’s no way of re-binding that default argument name in the function’s closure. When function is executed multiple times with its default value the value will change between executions, possibly leading to strange side effects. For more information see: http://satran.in/2012/01/12/python-dangerous-default-value-as-argument.html --- src/pylorax/imgutils.py | 31 ++++++++++++++++++++----------- src/pylorax/ltmpl.py | 8 ++++++-- src/pylorax/treebuilder.py | 8 ++++++-- src/sbin/lorax | 11 +++++++++-- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/pylorax/imgutils.py b/src/pylorax/imgutils.py index fc38d5f3..a9c31b8d 100644 --- a/src/pylorax/imgutils.py +++ b/src/pylorax/imgutils.py @@ -35,11 +35,13 @@ from pylorax.executils import runcmd, runcmd_output ######## Functions for making container images (cpio, tar, squashfs) ########## -def compress(command, rootdir, outfile, compression="xz", compressargs=["-9"]): +def compress(command, rootdir, outfile, compression="xz", compressargs=None): '''Make a compressed archive of the given rootdir. command is a list of the archiver commands to run compression should be "xz", "gzip", "lzma", "bzip2", or None. compressargs will be used on the compression commandline.''' + if compressargs is None: + compressargs = ["-9"] if compression not in (None, "xz", "gzip", "lzma", "bzip2"): raise ValueError, "Unknown compression type %s" % compression if compression == "xz": @@ -68,17 +70,18 @@ def compress(command, rootdir, outfile, compression="xz", compressargs=["-9"]): [p.kill() for p in (find, archive, comp) if p] return 1 -def mkcpio(rootdir, outfile, compression="xz", compressargs=["-9"]): +def mkcpio(rootdir, outfile, compression="xz", compressargs=None): return compress(["cpio", "--null", "--quiet", "-H", "newc", "-o"], rootdir, outfile, compression, compressargs) -def mktar(rootdir, outfile, compression="xz", compressargs=["-9"]): - compressargs = compressargs or ["-9"] +def mktar(rootdir, outfile, compression="xz", compressargs=None): return compress(["tar", "--no-recursion", "--selinux", "--acls", "--xattrs", "-cf-", "--null", "-T-"], rootdir, outfile, compression, compressargs) -def mksquashfs(rootdir, outfile, compression="default", compressargs=[]): +def mksquashfs(rootdir, outfile, compression="default", compressargs=None): '''Make a squashfs image containing the given rootdir.''' + if compressargs is None: + compressargs = [] if compression != "default": compressargs = ["-comp", compression] + compressargs return execWithRedirect("mksquashfs", [rootdir, outfile] + compressargs) @@ -303,7 +306,9 @@ def round_to_blocks(size, blocksize): return size # TODO: move filesystem data outside this function -def estimate_size(rootdir, graft={}, fstype=None, blocksize=4096, overhead=128): +def estimate_size(rootdir, graft=None, fstype=None, blocksize=4096, overhead=128): + if graft is None: + graft = {} getsize = lambda f: os.lstat(f).st_size if fstype == "btrfs": overhead = 64*1024 # don't worry, it's all sparse @@ -422,12 +427,16 @@ class PartitionMount(object): ######## Functions for making filesystem images ########################## -def mkfsimage(fstype, rootdir, outfile, size=None, mkfsargs=[], mountargs="", graft={}): +def mkfsimage(fstype, rootdir, outfile, size=None, mkfsargs=None, mountargs="", graft=None): '''Generic filesystem image creation function. fstype should be a filesystem type - "mkfs.${fstype}" must exist. graft should be a dict: {"some/path/in/image": "local/file/or/dir"}; if the path ends with a '/' it's assumed to be a directory. Will raise CalledProcessError if something goes wrong.''' + if mkfsargs is None: + mkfsargs = [] + if graft is None: + graft = {} preserve = (fstype not in ("msdos", "vfat")) if not size: size = estimate_size(rootdir, graft, fstype) @@ -448,18 +457,18 @@ def mkfsimage(fstype, rootdir, outfile, size=None, mkfsargs=[], mountargs="", gr runcmd(["sync"]) # convenience functions with useful defaults -def mkdosimg(rootdir, outfile, size=None, label="", mountargs="shortname=winnt,umask=0077", graft={}): +def mkdosimg(rootdir, outfile, size=None, label="", mountargs="shortname=winnt,umask=0077", graft=None): mkfsimage("msdos", rootdir, outfile, size, mountargs=mountargs, mkfsargs=["-n", label], graft=graft) -def mkext4img(rootdir, outfile, size=None, label="", mountargs="", graft={}): +def mkext4img(rootdir, outfile, size=None, label="", mountargs="", graft=None): mkfsimage("ext4", rootdir, outfile, size, mountargs=mountargs, mkfsargs=["-L", label, "-b", "1024", "-m", "0"], graft=graft) -def mkbtrfsimg(rootdir, outfile, size=None, label="", mountargs="", graft={}): +def mkbtrfsimg(rootdir, outfile, size=None, label="", mountargs="", graft=None): mkfsimage("btrfs", rootdir, outfile, size, mountargs=mountargs, mkfsargs=["-L", label], graft=graft) -def mkhfsimg(rootdir, outfile, size=None, label="", mountargs="", graft={}): +def mkhfsimg(rootdir, outfile, size=None, label="", mountargs="", graft=None): mkfsimage("hfsplus", rootdir, outfile, size, mountargs=mountargs, mkfsargs=["-v", label], graft=graft) diff --git a/src/pylorax/ltmpl.py b/src/pylorax/ltmpl.py index af15bbc3..364ea221 100644 --- a/src/pylorax/ltmpl.py +++ b/src/pylorax/ltmpl.py @@ -40,7 +40,9 @@ import sys, traceback import struct class LoraxTemplate(object): - def __init__(self, directories=["/usr/share/lorax"]): + def __init__(self, directories=None): + if directories is None: + directories = ["/usr/share/lorax"] # we have to add ["/"] to the template lookup directories or the # file includes won't work properly for absolute paths self.directories = ["/"] + directories @@ -148,7 +150,9 @@ class LoraxTemplateRunner(object): * Commands should raise exceptions for errors - don't use sys.exit() ''' def __init__(self, inroot, outroot, yum_obj=None, fatalerrors=True, - templatedir=None, defaults={}): + templatedir=None, defaults=None): + if defaults is None: + defaults = {} self.inroot = inroot self.outroot = outroot self.yum = yum_obj diff --git a/src/pylorax/treebuilder.py b/src/pylorax/treebuilder.py index d34fcf1e..ae86e294 100644 --- a/src/pylorax/treebuilder.py +++ b/src/pylorax/treebuilder.py @@ -156,7 +156,9 @@ class RuntimeBuilder(object): runcmd(["depmod", "-a", "-F", ksyms, "-b", root, kver]) generate_module_info(moddir+kver, outfile=moddir+"module-info") - def create_runtime(self, outfile="/var/tmp/squashfs.img", compression="xz", compressargs=[], size=2): + def create_runtime(self, outfile="/var/tmp/squashfs.img", compression="xz", compressargs=None, size=2): + if compressargs is None: + compressargs = [] # make live rootfs image - must be named "LiveOS/rootfs.img" for dracut workdir = joinpaths(os.path.dirname(outfile), "runtime-workdir") os.makedirs(joinpaths(workdir, "LiveOS")) @@ -192,13 +194,15 @@ class TreeBuilder(object): def kernels(self): return findkernels(root=self.vars.inroot) - def rebuild_initrds(self, add_args=[], backup="", prefix=""): + def rebuild_initrds(self, add_args=None, backup="", prefix=""): '''Rebuild all the initrds in the tree. If backup is specified, each initrd will be renamed with backup as a suffix before rebuilding. If backup is empty, the existing initrd files will be overwritten. If suffix is specified, the existing initrd is untouched and a new image is built with the filename "${prefix}-${kernel.version}.img" ''' + if add_args is None: + add_args = [] dracut = ["dracut", "--nomdadmconf", "--nolvmconf"] + add_args if not backup: dracut.append("--force") diff --git a/src/sbin/lorax b/src/sbin/lorax index 75e0faaf..08eb7054 100755 --- a/src/sbin/lorax +++ b/src/sbin/lorax @@ -244,8 +244,8 @@ def main(args): user_dracut_args=opts.dracut_args) -def get_yum_base_object(installroot, repositories, mirrorlists=[], repo_files=[], - tempdir="/var/tmp", proxy=None, excludepkgs=[], +def get_yum_base_object(installroot, repositories, mirrorlists=None, repo_files=None, + tempdir="/var/tmp", proxy=None, excludepkgs=None, sslverify=True, releasever=None): def sanitize_repo(repo): @@ -257,6 +257,13 @@ def get_yum_base_object(installroot, repositories, mirrorlists=[], repo_files=[] else: return None + if mirrorlists is None: + mirrorlists = [] + if repo_files is None: + repo_files = [] + if excludepkgs is None: + excludepkgs = [] + # sanitize the repositories repositories = map(sanitize_repo, repositories) mirrorlists = map(sanitize_repo, mirrorlists)