From 70adba89919bf5ac520b3f381a0d81e058e9f4fb Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Mon, 29 Apr 2024 22:57:08 +0200
Subject: [PATCH] Convert some run_command() string args to lists (#8264)

* Convert some run_command() string args to lists.

* Change run_command with pipe and shell to Python code.

* Add changelog.

* Simplify syntax.

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

---------

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
---
 changelogs/fragments/8264-run_command.yml | 14 ++++
 plugins/modules/aix_lvol.py               | 19 +++--
 plugins/modules/apt_rpm.py                | 18 ++---
 plugins/modules/btrfs_subvolume.py        |  9 +--
 plugins/modules/installp.py               | 13 ++--
 plugins/modules/lvg.py                    | 22 +++---
 plugins/modules/lvol.py                   | 84 +++++++++++------------
 plugins/modules/macports.py               | 12 ++--
 plugins/modules/parted.py                 |  9 +--
 plugins/modules/pkgin.py                  | 25 +++----
 plugins/modules/portinstall.py            | 26 +++----
 plugins/modules/slackpkg.py               | 18 +++--
 plugins/modules/svr4pkg.py                |  2 +-
 plugins/modules/swdepot.py                | 17 +++--
 14 files changed, 144 insertions(+), 144 deletions(-)
 create mode 100644 changelogs/fragments/8264-run_command.yml

diff --git a/changelogs/fragments/8264-run_command.yml b/changelogs/fragments/8264-run_command.yml
new file mode 100644
index 0000000000..dd66cd6123
--- /dev/null
+++ b/changelogs/fragments/8264-run_command.yml
@@ -0,0 +1,14 @@
+minor_changes:
+  - "aix_lvol - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "apt_rpm - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "btrfs_subvolume - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "installp - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "lvg - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "lvol - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "macports - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "parted - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "pkgin - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "portinstall - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "slackpkg - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "svr4pkg - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
+  - "swdepot - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)."
diff --git a/plugins/modules/aix_lvol.py b/plugins/modules/aix_lvol.py
index 1e7b425687..7d0fb1ee09 100644
--- a/plugins/modules/aix_lvol.py
+++ b/plugins/modules/aix_lvol.py
@@ -240,8 +240,6 @@ def main():
     state = module.params['state']
     pvs = module.params['pvs']
 
-    pv_list = ' '.join(pvs)
-
     if policy == 'maximum':
         lv_policy = 'x'
     else:
@@ -249,16 +247,16 @@ def main():
 
     # Add echo command when running in check-mode
     if module.check_mode:
-        test_opt = 'echo '
+        test_opt = [module.get_bin_path("echo", required=True)]
     else:
-        test_opt = ''
+        test_opt = []
 
     # check if system commands are available
     lsvg_cmd = module.get_bin_path("lsvg", required=True)
     lslv_cmd = module.get_bin_path("lslv", required=True)
 
     # Get information on volume group requested
-    rc, vg_info, err = module.run_command("%s %s" % (lsvg_cmd, vg))
+    rc, vg_info, err = module.run_command([lsvg_cmd, vg])
 
     if rc != 0:
         if state == 'absent':
@@ -273,8 +271,7 @@ def main():
         lv_size = round_ppsize(convert_size(module, size), base=this_vg['pp_size'])
 
     # Get information on logical volume requested
-    rc, lv_info, err = module.run_command(
-        "%s %s" % (lslv_cmd, lv))
+    rc, lv_info, err = module.run_command([lslv_cmd, lv])
 
     if rc != 0:
         if state == 'absent':
@@ -296,7 +293,7 @@ def main():
             # create LV
             mklv_cmd = module.get_bin_path("mklv", required=True)
 
-            cmd = "%s %s -t %s -y %s -c %s  -e %s %s %s %sM %s" % (test_opt, mklv_cmd, lv_type, lv, copies, lv_policy, opts, vg, lv_size, pv_list)
+            cmd = test_opt + [mklv_cmd, "-t", lv_type, "-y", lv, "-c", copies, "-e", lv_policy, opts, vg, "%sM" % (lv_size, )] + pvs
             rc, out, err = module.run_command(cmd)
             if rc == 0:
                 module.exit_json(changed=True, msg="Logical volume %s created." % lv)
@@ -306,7 +303,7 @@ def main():
         if state == 'absent':
             # remove LV
             rmlv_cmd = module.get_bin_path("rmlv", required=True)
-            rc, out, err = module.run_command("%s %s -f %s" % (test_opt, rmlv_cmd, this_lv['name']))
+            rc, out, err = module.run_command(test_opt + [rmlv_cmd, "-f", this_lv['name']])
             if rc == 0:
                 module.exit_json(changed=True, msg="Logical volume %s deleted." % lv)
             else:
@@ -315,7 +312,7 @@ def main():
             if this_lv['policy'] != policy:
                 # change lv allocation policy
                 chlv_cmd = module.get_bin_path("chlv", required=True)
-                rc, out, err = module.run_command("%s %s -e %s %s" % (test_opt, chlv_cmd, lv_policy, this_lv['name']))
+                rc, out, err = module.run_command(test_opt + [chlv_cmd, "-e", lv_policy, this_lv['name']])
                 if rc == 0:
                     module.exit_json(changed=True, msg="Logical volume %s policy changed: %s." % (lv, policy))
                 else:
@@ -331,7 +328,7 @@ def main():
             # resize LV based on absolute values
             if int(lv_size) > this_lv['size']:
                 extendlv_cmd = module.get_bin_path("extendlv", required=True)
-                cmd = "%s %s %s %sM" % (test_opt, extendlv_cmd, lv, lv_size - this_lv['size'])
+                cmd = test_opt + [extendlv_cmd, lv, "%sM" % (lv_size - this_lv['size'], )]
                 rc, out, err = module.run_command(cmd)
                 if rc == 0:
                     module.exit_json(changed=True, msg="Logical volume %s size extended to %sMB." % (lv, lv_size))
diff --git a/plugins/modules/apt_rpm.py b/plugins/modules/apt_rpm.py
index 03b87e78f0..07da307633 100644
--- a/plugins/modules/apt_rpm.py
+++ b/plugins/modules/apt_rpm.py
@@ -170,7 +170,7 @@ def local_rpm_package_name(path):
 def query_package(module, name):
     # rpm -q returns 0 if the package is installed,
     # 1 if it is not installed
-    rc, out, err = module.run_command("%s -q %s" % (RPM_PATH, name))
+    rc, out, err = module.run_command([RPM_PATH, "-q", name])
     if rc == 0:
         return True
     else:
@@ -203,7 +203,7 @@ def query_package_provides(module, name, allow_upgrade=False):
 
         name = local_rpm_package_name(name)
 
-    rc, out, err = module.run_command("%s -q --provides %s" % (RPM_PATH, name))
+    rc, out, err = module.run_command([RPM_PATH, "-q", "--provides", name])
     if rc == 0:
         if not allow_upgrade:
             return True
@@ -253,7 +253,7 @@ def remove_packages(module, packages):
         if not query_package(module, package):
             continue
 
-        rc, out, err = module.run_command("%s -y remove %s" % (APT_PATH, package), environ_update={"LANG": "C"})
+        rc, out, err = module.run_command([APT_PATH, "-y", "remove", package], environ_update={"LANG": "C"})
 
         if rc != 0:
             module.fail_json(msg="failed to remove %s: %s" % (package, err))
@@ -271,14 +271,14 @@ def install_packages(module, pkgspec, allow_upgrade=False):
     if pkgspec is None:
         return (False, "Empty package list")
 
-    packages = ""
+    packages = []
     for package in pkgspec:
         if not query_package_provides(module, package, allow_upgrade=allow_upgrade):
-            packages += "'%s' " % package
+            packages.append(package)
 
-    if len(packages) != 0:
-
-        rc, out, err = module.run_command("%s -y install %s" % (APT_PATH, packages), environ_update={"LANG": "C"})
+    if packages:
+        command = [APT_PATH, "-y", "install"] + packages
+        rc, out, err = module.run_command(command, environ_update={"LANG": "C"})
 
         installed = True
         for package in pkgspec:
@@ -287,7 +287,7 @@ def install_packages(module, pkgspec, allow_upgrade=False):
 
         # apt-rpm always have 0 for exit code if --force is used
         if rc or not installed:
-            module.fail_json(msg="'apt-get -y install %s' failed: %s" % (packages, err))
+            module.fail_json(msg="'%s' failed: %s" % (" ".join(command), err))
         else:
             return (True, "%s present(s)" % packages)
     else:
diff --git a/plugins/modules/btrfs_subvolume.py b/plugins/modules/btrfs_subvolume.py
index 864bb65a66..35327bfe02 100644
--- a/plugins/modules/btrfs_subvolume.py
+++ b/plugins/modules/btrfs_subvolume.py
@@ -572,10 +572,7 @@ class BtrfsSubvolumeModule(object):
         self.__temporary_mounts[cache_key] = mountpoint
 
         mount = self.module.get_bin_path("mount", required=True)
-        command = "%s -o noatime,subvolid=%d %s %s " % (mount,
-                                                        subvolid,
-                                                        device,
-                                                        mountpoint)
+        command = [mount, "-o", "noatime,subvolid=%d" % subvolid, device, mountpoint]
         result = self.module.run_command(command, check_rc=True)
 
         return mountpoint
@@ -586,10 +583,10 @@ class BtrfsSubvolumeModule(object):
 
     def __cleanup_mount(self, mountpoint):
         umount = self.module.get_bin_path("umount", required=True)
-        result = self.module.run_command("%s %s" % (umount, mountpoint))
+        result = self.module.run_command([umount, mountpoint])
         if result[0] == 0:
             rmdir = self.module.get_bin_path("rmdir", required=True)
-            self.module.run_command("%s %s" % (rmdir, mountpoint))
+            self.module.run_command([rmdir, mountpoint])
 
     # Format and return results
     def get_results(self):
diff --git a/plugins/modules/installp.py b/plugins/modules/installp.py
index 4b5a6949c6..1531d2cad2 100644
--- a/plugins/modules/installp.py
+++ b/plugins/modules/installp.py
@@ -106,7 +106,7 @@ def _check_new_pkg(module, package, repository_path):
 
     if os.path.isdir(repository_path):
         installp_cmd = module.get_bin_path('installp', True)
-        rc, package_result, err = module.run_command("%s -l -MR -d %s" % (installp_cmd, repository_path))
+        rc, package_result, err = module.run_command([installp_cmd, "-l", "-MR", "-d", repository_path])
         if rc != 0:
             module.fail_json(msg="Failed to run installp.", rc=rc, err=err)
 
@@ -142,7 +142,7 @@ def _check_installed_pkg(module, package, repository_path):
     """
 
     lslpp_cmd = module.get_bin_path('lslpp', True)
-    rc, lslpp_result, err = module.run_command("%s -lcq %s*" % (lslpp_cmd, package))
+    rc, lslpp_result, err = module.run_command([lslpp_cmd, "-lcq", "%s*" % (package, )])
 
     if rc == 1:
         package_state = ' '.join(err.split()[-2:])
@@ -173,7 +173,7 @@ def remove(module, installp_cmd, packages):
 
         if pkg_check:
             if not module.check_mode:
-                rc, remove_out, err = module.run_command("%s -u %s" % (installp_cmd, package))
+                rc, remove_out, err = module.run_command([installp_cmd, "-u", package])
                 if rc != 0:
                     module.fail_json(msg="Failed to run installp.", rc=rc, err=err)
             remove_count += 1
@@ -202,8 +202,8 @@ def install(module, installp_cmd, packages, repository_path, accept_license):
     already_installed_pkgs = {}
 
     accept_license_param = {
-        True: '-Y',
-        False: '',
+        True: ['-Y'],
+        False: [],
     }
 
     # Validate if package exists on repository path.
@@ -230,7 +230,8 @@ def install(module, installp_cmd, packages, repository_path, accept_license):
 
             else:
                 if not module.check_mode:
-                    rc, out, err = module.run_command("%s -a %s -X -d %s %s" % (installp_cmd, accept_license_param[accept_license], repository_path, package))
+                    rc, out, err = module.run_command(
+                        [installp_cmd, "-a"] + accept_license_param[accept_license] + ["-X", "-d", repository_path, package])
                     if rc != 0:
                         module.fail_json(msg="Failed to run installp", rc=rc, err=err)
                 installed_pkgs.append(package)
diff --git a/plugins/modules/lvg.py b/plugins/modules/lvg.py
index 8a6384369a..7ff7e3a2e7 100644
--- a/plugins/modules/lvg.py
+++ b/plugins/modules/lvg.py
@@ -179,7 +179,7 @@ def parse_vgs(data):
 def find_mapper_device_name(module, dm_device):
     dmsetup_cmd = module.get_bin_path('dmsetup', True)
     mapper_prefix = '/dev/mapper/'
-    rc, dm_name, err = module.run_command("%s info -C --noheadings -o name %s" % (dmsetup_cmd, dm_device))
+    rc, dm_name, err = module.run_command([dmsetup_cmd, "info", "-C", "--noheadings", "-o", "name", dm_device])
     if rc != 0:
         module.fail_json(msg="Failed executing dmsetup command.", rc=rc, err=err)
     mapper_device = mapper_prefix + dm_name.rstrip()
@@ -204,7 +204,7 @@ def find_vg(module, vg):
     if not vg:
         return None
     vgs_cmd = module.get_bin_path('vgs', True)
-    dummy, current_vgs, dummy = module.run_command("%s --noheadings -o vg_name,pv_count,lv_count --separator ';'" % vgs_cmd, check_rc=True)
+    dummy, current_vgs, dummy = module.run_command([vgs_cmd, "--noheadings", "-o", "vg_name,pv_count,lv_count", "--separator", ";"], check_rc=True)
 
     vgs = parse_vgs(current_vgs)
 
@@ -431,10 +431,10 @@ def main():
                 for x in itertools.chain(dev_list, module.params['pvs'])
             )
             pvs_filter_vg_name = 'vg_name = {0}'.format(vg)
-            pvs_filter = "--select '{0} || {1}' ".format(pvs_filter_pv_name, pvs_filter_vg_name)
+            pvs_filter = ["--select", "{0} || {1}".format(pvs_filter_pv_name, pvs_filter_vg_name)]
         else:
-            pvs_filter = ''
-        rc, current_pvs, err = module.run_command("%s --noheadings -o pv_name,vg_name --separator ';' %s" % (pvs_cmd, pvs_filter))
+            pvs_filter = []
+        rc, current_pvs, err = module.run_command([pvs_cmd, "--noheadings", "-o", "pv_name,vg_name", "--separator", ";"] + pvs_filter)
         if rc != 0:
             module.fail_json(msg="Failed executing pvs command.", rc=rc, err=err)
 
@@ -473,7 +473,7 @@ def main():
                 if this_vg['lv_count'] == 0 or force:
                     # remove VG
                     vgremove_cmd = module.get_bin_path('vgremove', True)
-                    rc, dummy, err = module.run_command("%s --force %s" % (vgremove_cmd, vg))
+                    rc, dummy, err = module.run_command([vgremove_cmd, "--force", vg])
                     if rc == 0:
                         module.exit_json(changed=True)
                     else:
@@ -509,7 +509,6 @@ def main():
                     changed = True
                 else:
                     if devs_to_add:
-                        devs_to_add_string = ' '.join(devs_to_add)
                         # create PV
                         pvcreate_cmd = module.get_bin_path('pvcreate', True)
                         for current_dev in devs_to_add:
@@ -520,21 +519,20 @@ def main():
                                 module.fail_json(msg="Creating physical volume '%s' failed" % current_dev, rc=rc, err=err)
                         # add PV to our VG
                         vgextend_cmd = module.get_bin_path('vgextend', True)
-                        rc, dummy, err = module.run_command("%s %s %s" % (vgextend_cmd, vg, devs_to_add_string))
+                        rc, dummy, err = module.run_command([vgextend_cmd, vg] + devs_to_add)
                         if rc == 0:
                             changed = True
                         else:
-                            module.fail_json(msg="Unable to extend %s by %s." % (vg, devs_to_add_string), rc=rc, err=err)
+                            module.fail_json(msg="Unable to extend %s by %s." % (vg, ' '.join(devs_to_add)), rc=rc, err=err)
 
                     # remove some PV from our VG
                     if devs_to_remove:
-                        devs_to_remove_string = ' '.join(devs_to_remove)
                         vgreduce_cmd = module.get_bin_path('vgreduce', True)
-                        rc, dummy, err = module.run_command("%s --force %s %s" % (vgreduce_cmd, vg, devs_to_remove_string))
+                        rc, dummy, err = module.run_command([vgreduce_cmd, "--force", vg] + devs_to_remove)
                         if rc == 0:
                             changed = True
                         else:
-                            module.fail_json(msg="Unable to reduce %s by %s." % (vg, devs_to_remove_string), rc=rc, err=err)
+                            module.fail_json(msg="Unable to reduce %s by %s." % (vg, ' '.join(devs_to_remove)), rc=rc, err=err)
 
     module.exit_json(changed=changed)
 
diff --git a/plugins/modules/lvol.py b/plugins/modules/lvol.py
index a2a870260a..3a2f5c7cdd 100644
--- a/plugins/modules/lvol.py
+++ b/plugins/modules/lvol.py
@@ -236,6 +236,7 @@ EXAMPLES = '''
 '''
 
 import re
+import shlex
 
 from ansible.module_utils.basic import AnsibleModule
 
@@ -281,7 +282,7 @@ def parse_vgs(data):
 
 def get_lvm_version(module):
     ver_cmd = module.get_bin_path("lvm", required=True)
-    rc, out, err = module.run_command("%s version" % (ver_cmd))
+    rc, out, err = module.run_command([ver_cmd, "version"])
     if rc != 0:
         return None
     m = re.search(r"LVM version:\s+(\d+)\.(\d+)\.(\d+).*(\d{4}-\d{2}-\d{2})", out)
@@ -320,14 +321,14 @@ def main():
         module.fail_json(msg="Failed to get LVM version number")
     version_yesopt = mkversion(2, 2, 99)  # First LVM with the "--yes" option
     if version_found >= version_yesopt:
-        yesopt = "--yes"
+        yesopt = ["--yes"]
     else:
-        yesopt = ""
+        yesopt = []
 
     vg = module.params['vg']
     lv = module.params['lv']
     size = module.params['size']
-    opts = module.params['opts']
+    opts = shlex.split(module.params['opts'] or '')
     state = module.params['state']
     force = module.boolean(module.params['force'])
     shrink = module.boolean(module.params['shrink'])
@@ -338,21 +339,13 @@ def main():
     size_unit = 'm'
     size_operator = None
     snapshot = module.params['snapshot']
-    pvs = module.params['pvs']
-
-    if pvs is None:
-        pvs = ""
-    else:
-        pvs = " ".join(pvs)
-
-    if opts is None:
-        opts = ""
+    pvs = module.params['pvs'] or []
 
     # Add --test option when running in check-mode
     if module.check_mode:
-        test_opt = ' --test'
+        test_opt = ['--test']
     else:
-        test_opt = ''
+        test_opt = []
 
     if size:
         # LVEXTEND(8)/LVREDUCE(8) -l, -L options: Check for relative value for resizing
@@ -400,7 +393,7 @@ def main():
     # Get information on volume group requested
     vgs_cmd = module.get_bin_path("vgs", required=True)
     rc, current_vgs, err = module.run_command(
-        "%s --noheadings --nosuffix -o vg_name,size,free,vg_extent_size --units %s --separator ';' %s" % (vgs_cmd, unit.lower(), vg))
+        [vgs_cmd, "--noheadings", "--nosuffix", "-o", "vg_name,size,free,vg_extent_size", "--units", unit.lower(), "--separator", ";", vg])
 
     if rc != 0:
         if state == 'absent':
@@ -414,7 +407,7 @@ def main():
     # Get information on logical volume requested
     lvs_cmd = module.get_bin_path("lvs", required=True)
     rc, current_lvs, err = module.run_command(
-        "%s -a --noheadings --nosuffix -o lv_name,size,lv_attr --units %s --separator ';' %s" % (lvs_cmd, unit.lower(), vg))
+        [lvs_cmd, "-a", "--noheadings", "--nosuffix", "-o", "lv_name,size,lv_attr", "--units", unit.lower(), "--separator", ";", vg])
 
     if rc != 0:
         if state == 'absent':
@@ -474,20 +467,23 @@ def main():
 
             # create LV
             lvcreate_cmd = module.get_bin_path("lvcreate", required=True)
+            cmd = [lvcreate_cmd] + test_opt + yesopt
             if snapshot is not None:
                 if size:
-                    cmd = "%s %s %s -%s %s%s -s -n %s %s %s/%s" % (lvcreate_cmd, test_opt, yesopt, size_opt, size, size_unit, snapshot, opts, vg, lv)
-                else:
-                    cmd = "%s %s %s -s -n %s %s %s/%s" % (lvcreate_cmd, test_opt, yesopt, snapshot, opts, vg, lv)
-            elif thinpool and lv:
-                if size_opt == 'l':
-                    module.fail_json(changed=False, msg="Thin volume sizing with percentage not supported.")
-                size_opt = 'V'
-                cmd = "%s %s %s -n %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, thinpool)
-            elif thinpool and not lv:
-                cmd = "%s %s %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, test_opt, yesopt, size_opt, size, size_unit, opts, vg, thinpool)
+                    cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)]
+                cmd += ["-s", "-n", snapshot] + opts + ["%s/%s" % (vg, lv)]
+            elif thinpool:
+                if lv:
+                    if size_opt == 'l':
+                        module.fail_json(changed=False, msg="Thin volume sizing with percentage not supported.")
+                    size_opt = 'V'
+                    cmd += ["-n", lv]
+                cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)]
+                cmd += opts + ["-T", "%s/%s" % (vg, thinpool)]
             else:
-                cmd = "%s %s %s -n %s -%s %s%s %s %s %s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, pvs)
+                cmd += ["-n", lv]
+                cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)]
+                cmd += opts + [vg] + pvs
             rc, dummy, err = module.run_command(cmd)
             if rc == 0:
                 changed = True
@@ -499,7 +495,7 @@ def main():
             if not force:
                 module.fail_json(msg="Sorry, no removal of logical volume %s without force=true." % (this_lv['name']))
             lvremove_cmd = module.get_bin_path("lvremove", required=True)
-            rc, dummy, err = module.run_command("%s %s --force %s/%s" % (lvremove_cmd, test_opt, vg, this_lv['name']))
+            rc, dummy, err = module.run_command([lvremove_cmd] + test_opt + ["--force", "%s/%s" % (vg, this_lv['name'])])
             if rc == 0:
                 module.exit_json(changed=True)
             else:
@@ -527,7 +523,7 @@ def main():
 
             if this_lv['size'] < size_requested:
                 if (size_free > 0) and (size_free >= (size_requested - this_lv['size'])):
-                    tool = module.get_bin_path("lvextend", required=True)
+                    tool = [module.get_bin_path("lvextend", required=True)]
                 else:
                     module.fail_json(
                         msg="Logical Volume %s could not be extended. Not enough free space left (%s%s required / %s%s available)" %
@@ -539,16 +535,17 @@ def main():
                 elif not force:
                     module.fail_json(msg="Sorry, no shrinking of %s without force=true" % (this_lv['name']))
                 else:
-                    tool = module.get_bin_path("lvreduce", required=True)
-                    tool = '%s %s' % (tool, '--force')
+                    tool = [module.get_bin_path("lvreduce", required=True), '--force']
 
             if tool:
                 if resizefs:
-                    tool = '%s %s' % (tool, '--resizefs')
+                    tool += ['--resizefs']
+                cmd = tool + test_opt
                 if size_operator:
-                    cmd = "%s %s -%s %s%s%s %s/%s %s" % (tool, test_opt, size_opt, size_operator, size, size_unit, vg, this_lv['name'], pvs)
+                    cmd += ["-%s" % size_opt, "%s%s%s" % (size_operator, size, size_unit)]
                 else:
-                    cmd = "%s %s -%s %s%s %s/%s %s" % (tool, test_opt, size_opt, size, size_unit, vg, this_lv['name'], pvs)
+                    cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)]
+                cmd += ["%s/%s" % (vg, this_lv['name'])] + pvs
                 rc, out, err = module.run_command(cmd)
                 if "Reached maximum COW size" in out:
                     module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out)
@@ -566,23 +563,24 @@ def main():
             # resize LV based on absolute values
             tool = None
             if float(size) > this_lv['size'] or size_operator == '+':
-                tool = module.get_bin_path("lvextend", required=True)
+                tool = [module.get_bin_path("lvextend", required=True)]
             elif shrink and float(size) < this_lv['size'] or size_operator == '-':
                 if float(size) == 0:
                     module.fail_json(msg="Sorry, no shrinking of %s to 0 permitted." % (this_lv['name']))
                 if not force:
                     module.fail_json(msg="Sorry, no shrinking of %s without force=true." % (this_lv['name']))
                 else:
-                    tool = module.get_bin_path("lvreduce", required=True)
-                    tool = '%s %s' % (tool, '--force')
+                    tool = [module.get_bin_path("lvreduce", required=True), '--force']
 
             if tool:
                 if resizefs:
-                    tool = '%s %s' % (tool, '--resizefs')
+                    tool += ['--resizefs']
+                cmd = tool + test_opt
                 if size_operator:
-                    cmd = "%s %s -%s %s%s%s %s/%s %s" % (tool, test_opt, size_opt, size_operator, size, size_unit, vg, this_lv['name'], pvs)
+                    cmd += ["-%s" % size_opt, "%s%s%s" % (size_operator, size, size_unit)]
                 else:
-                    cmd = "%s %s -%s %s%s %s/%s %s" % (tool, test_opt, size_opt, size, size_unit, vg, this_lv['name'], pvs)
+                    cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)]
+                cmd += ["%s/%s" % (vg, this_lv['name'])] + pvs
                 rc, out, err = module.run_command(cmd)
                 if "Reached maximum COW size" in out:
                     module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out)
@@ -598,14 +596,14 @@ def main():
     if this_lv is not None:
         if active:
             lvchange_cmd = module.get_bin_path("lvchange", required=True)
-            rc, dummy, err = module.run_command("%s -ay %s/%s" % (lvchange_cmd, vg, this_lv['name']))
+            rc, dummy, err = module.run_command([lvchange_cmd, "-ay", "%s/%s" % (vg, this_lv['name'])])
             if rc == 0:
                 module.exit_json(changed=((not this_lv['active']) or changed), vg=vg, lv=this_lv['name'], size=this_lv['size'])
             else:
                 module.fail_json(msg="Failed to activate logical volume %s" % (lv), rc=rc, err=err)
         else:
             lvchange_cmd = module.get_bin_path("lvchange", required=True)
-            rc, dummy, err = module.run_command("%s -an %s/%s" % (lvchange_cmd, vg, this_lv['name']))
+            rc, dummy, err = module.run_command([lvchange_cmd, "-an", "%s/%s" % (vg, this_lv['name'])])
             if rc == 0:
                 module.exit_json(changed=(this_lv['active'] or changed), vg=vg, lv=this_lv['name'], size=this_lv['size'])
             else:
diff --git a/plugins/modules/macports.py b/plugins/modules/macports.py
index e81fb9142c..cd620687d7 100644
--- a/plugins/modules/macports.py
+++ b/plugins/modules/macports.py
@@ -111,7 +111,7 @@ from ansible.module_utils.basic import AnsibleModule
 def selfupdate(module, port_path):
     """ Update Macports and the ports tree. """
 
-    rc, out, err = module.run_command("%s -v selfupdate" % port_path)
+    rc, out, err = module.run_command([port_path, "-v", "selfupdate"])
 
     if rc == 0:
         updated = any(
@@ -135,7 +135,7 @@ def selfupdate(module, port_path):
 def upgrade(module, port_path):
     """ Upgrade outdated ports. """
 
-    rc, out, err = module.run_command("%s upgrade outdated" % port_path)
+    rc, out, err = module.run_command([port_path, "upgrade", "outdated"])
 
     # rc is 1 when nothing to upgrade so check stdout first.
     if out.strip() == "Nothing to upgrade.":
@@ -182,7 +182,7 @@ def remove_ports(module, port_path, ports, stdout, stderr):
         if not query_port(module, port_path, port):
             continue
 
-        rc, out, err = module.run_command("%s uninstall %s" % (port_path, port))
+        rc, out, err = module.run_command([port_path, "uninstall", port])
         stdout += out
         stderr += err
         if query_port(module, port_path, port):
@@ -206,7 +206,7 @@ def install_ports(module, port_path, ports, variant, stdout, stderr):
         if query_port(module, port_path, port):
             continue
 
-        rc, out, err = module.run_command("%s install %s %s" % (port_path, port, variant))
+        rc, out, err = module.run_command([port_path, "install", port, variant])
         stdout += out
         stderr += err
         if not query_port(module, port_path, port):
@@ -232,7 +232,7 @@ def activate_ports(module, port_path, ports, stdout, stderr):
         if query_port(module, port_path, port, state="active"):
             continue
 
-        rc, out, err = module.run_command("%s activate %s" % (port_path, port))
+        rc, out, err = module.run_command([port_path, "activate", port])
         stdout += out
         stderr += err
 
@@ -259,7 +259,7 @@ def deactivate_ports(module, port_path, ports, stdout, stderr):
         if not query_port(module, port_path, port, state="active"):
             continue
 
-        rc, out, err = module.run_command("%s deactivate %s" % (port_path, port))
+        rc, out, err = module.run_command([port_path, "deactivate", port])
         stdout += out
         stderr += err
         if query_port(module, port_path, port, state="active"):
diff --git a/plugins/modules/parted.py b/plugins/modules/parted.py
index 382e47a475..b3616a8ecd 100644
--- a/plugins/modules/parted.py
+++ b/plugins/modules/parted.py
@@ -480,12 +480,12 @@ def get_device_info(device, unit):
     if label_needed:
         return get_unlabeled_device_info(device, unit)
 
-    command = "%s -s -m %s -- unit '%s' print" % (parted_exec, device, unit)
+    command = [parted_exec, "-s", "-m", device, "--", "unit", unit, "print"]
     rc, out, err = module.run_command(command)
     if rc != 0 and 'unrecognised disk label' not in err:
         module.fail_json(msg=(
             "Error while getting device information with parted "
-            "script: '%s'" % command),
+            "script: '%s'" % " ".join(command)),
             rc=rc, out=out, err=err
         )
 
@@ -506,7 +506,7 @@ def check_parted_label(device):
         return False
 
     # Older parted versions return a message in the stdout and RC > 0.
-    rc, out, err = module.run_command("%s -s -m %s print" % (parted_exec, device))
+    rc, out, err = module.run_command([parted_exec, "-s", "-m", device, "print"])
     if rc != 0 and 'unrecognised disk label' in out.lower():
         return True
 
@@ -546,7 +546,7 @@ def parted_version():
     """
     global module, parted_exec  # pylint: disable=global-variable-not-assigned
 
-    rc, out, err = module.run_command("%s --version" % parted_exec)
+    rc, out, err = module.run_command([parted_exec, "--version"])
     if rc != 0:
         module.fail_json(
             msg="Failed to get parted version.", rc=rc, out=out, err=err
@@ -580,6 +580,7 @@ def parted(script, device, align):
         script_option = '-s'
 
     if script and not module.check_mode:
+        # TODO: convert run_comand() argument to list!
         command = "%s %s -m %s %s -- %s" % (parted_exec, script_option, align_option, device, script)
         rc, out, err = module.run_command(command)
 
diff --git a/plugins/modules/pkgin.py b/plugins/modules/pkgin.py
index 5b2e478b8c..8b29655d37 100644
--- a/plugins/modules/pkgin.py
+++ b/plugins/modules/pkgin.py
@@ -145,18 +145,18 @@ def query_package(module, name):
     """
 
     # test whether '-p' (parsable) flag is supported.
-    rc, out, err = module.run_command("%s -p -v" % PKGIN_PATH)
+    rc, out, err = module.run_command([PKGIN_PATH, "-p", "-v"])
 
     if rc == 0:
-        pflag = '-p'
+        pflag = ['-p']
         splitchar = ';'
     else:
-        pflag = ''
+        pflag = []
         splitchar = ' '
 
     # Use "pkgin search" to find the package. The regular expression will
     # only match on the complete name.
-    rc, out, err = module.run_command("%s %s search \"^%s$\"" % (PKGIN_PATH, pflag, name))
+    rc, out, err = module.run_command([PKGIN_PATH] + pflag + ["search", "^%s$" % name])
 
     # rc will not be 0 unless the search was a success
     if rc == 0:
@@ -234,22 +234,19 @@ def format_pkgin_command(module, command, package=None):
     # an empty string. Some commands (e.g. 'update') will ignore extra
     # arguments, however this behaviour cannot be relied on for others.
     if package is None:
-        package = ""
+        packages = []
+    else:
+        packages = [package]
 
     if module.params["force"]:
-        force = "-F"
+        force = ["-F"]
     else:
-        force = ""
-
-    vars = {"pkgin": PKGIN_PATH,
-            "command": command,
-            "package": package,
-            "force": force}
+        force = []
 
     if module.check_mode:
-        return "%(pkgin)s -n %(command)s %(package)s" % vars
+        return [PKGIN_PATH, "-n", command] + packages
     else:
-        return "%(pkgin)s -y %(force)s %(command)s %(package)s" % vars
+        return [PKGIN_PATH, "-y"] + force + [command] + packages
 
 
 def remove_packages(module, packages):
diff --git a/plugins/modules/portinstall.py b/plugins/modules/portinstall.py
index e263b71813..59dafb1eb8 100644
--- a/plugins/modules/portinstall.py
+++ b/plugins/modules/portinstall.py
@@ -79,12 +79,13 @@ def query_package(module, name):
     if pkg_info_path:
         pkgng = False
         pkg_glob_path = module.get_bin_path('pkg_glob', True)
+        # TODO: convert run_comand() argument to list!
         rc, out, err = module.run_command("%s -e `pkg_glob %s`" % (pkg_info_path, shlex_quote(name)), use_unsafe_shell=True)
+        pkg_info_path = [pkg_info_path]
     else:
         pkgng = True
-        pkg_info_path = module.get_bin_path('pkg', True)
-        pkg_info_path = pkg_info_path + " info"
-        rc, out, err = module.run_command("%s %s" % (pkg_info_path, name))
+        pkg_info_path = [module.get_bin_path('pkg', True), "info"]
+        rc, out, err = module.run_command(pkg_info_path + [name])
 
     found = rc == 0
 
@@ -94,10 +95,7 @@ def query_package(module, name):
         # some package is installed
         name_without_digits = re.sub('[0-9]', '', name)
         if name != name_without_digits:
-            if pkgng:
-                rc, out, err = module.run_command("%s %s" % (pkg_info_path, name_without_digits))
-            else:
-                rc, out, err = module.run_command("%s %s" % (pkg_info_path, name_without_digits))
+            rc, out, err = module.run_command(pkg_info_path + [name_without_digits])
 
         found = rc == 0
 
@@ -107,13 +105,13 @@ def query_package(module, name):
 def matching_packages(module, name):
 
     ports_glob_path = module.get_bin_path('ports_glob', True)
-    rc, out, err = module.run_command("%s %s" % (ports_glob_path, name))
+    rc, out, err = module.run_command([ports_glob_path, name])
     # counts the number of packages found
     occurrences = out.count('\n')
     if occurrences == 0:
         name_without_digits = re.sub('[0-9]', '', name)
         if name != name_without_digits:
-            rc, out, err = module.run_command("%s %s" % (ports_glob_path, name_without_digits))
+            rc, out, err = module.run_command([ports_glob_path, name_without_digits])
             occurrences = out.count('\n')
     return occurrences
 
@@ -135,10 +133,12 @@ def remove_packages(module, packages):
         if not query_package(module, package):
             continue
 
+        # TODO: convert run_comand() argument to list!
         rc, out, err = module.run_command("%s `%s %s`" % (pkg_delete_path, pkg_glob_path, shlex_quote(package)), use_unsafe_shell=True)
 
         if query_package(module, package):
             name_without_digits = re.sub('[0-9]', '', package)
+            # TODO: convert run_comand() argument to list!
             rc, out, err = module.run_command("%s `%s %s`" % (pkg_delete_path, pkg_glob_path,
                                                               shlex_quote(name_without_digits)),
                                               use_unsafe_shell=True)
@@ -163,13 +163,13 @@ def install_packages(module, packages, use_packages):
     if not portinstall_path:
         pkg_path = module.get_bin_path('pkg', False)
         if pkg_path:
-            module.run_command("pkg install -y portupgrade")
+            module.run_command([pkg_path, "install", "-y", "portupgrade"])
         portinstall_path = module.get_bin_path('portinstall', True)
 
     if use_packages:
-        portinstall_params = "--use-packages"
+        portinstall_params = ["--use-packages"]
     else:
-        portinstall_params = ""
+        portinstall_params = []
 
     for package in packages:
         if query_package(module, package):
@@ -178,7 +178,7 @@ def install_packages(module, packages, use_packages):
         # TODO: check how many match
         matches = matching_packages(module, package)
         if matches == 1:
-            rc, out, err = module.run_command("%s --batch %s %s" % (portinstall_path, portinstall_params, package))
+            rc, out, err = module.run_command([portinstall_path, "--batch"] + portinstall_params + [package])
             if not query_package(module, package):
                 module.fail_json(msg="failed to install %s: %s" % (package, out))
         elif matches == 0:
diff --git a/plugins/modules/slackpkg.py b/plugins/modules/slackpkg.py
index e3d7a15429..9347db1591 100644
--- a/plugins/modules/slackpkg.py
+++ b/plugins/modules/slackpkg.py
@@ -106,9 +106,8 @@ def remove_packages(module, slackpkg_path, packages):
             continue
 
         if not module.check_mode:
-            rc, out, err = module.run_command("%s -default_answer=y -batch=on \
-                                              remove %s" % (slackpkg_path,
-                                                            package))
+            rc, out, err = module.run_command(
+                [slackpkg_path, "-default_answer=y", "-batch=on", "remove", package])
 
         if not module.check_mode and query_package(module, slackpkg_path,
                                                    package):
@@ -132,9 +131,8 @@ def install_packages(module, slackpkg_path, packages):
             continue
 
         if not module.check_mode:
-            rc, out, err = module.run_command("%s -default_answer=y -batch=on \
-                                              install %s" % (slackpkg_path,
-                                                             package))
+            rc, out, err = module.run_command(
+                [slackpkg_path, "-default_answer=y", "-batch=on", "install", package])
 
         if not module.check_mode and not query_package(module, slackpkg_path,
                                                        package):
@@ -155,9 +153,8 @@ def upgrade_packages(module, slackpkg_path, packages):
 
     for package in packages:
         if not module.check_mode:
-            rc, out, err = module.run_command("%s -default_answer=y -batch=on \
-                                              upgrade %s" % (slackpkg_path,
-                                                             package))
+            rc, out, err = module.run_command(
+                [slackpkg_path, "-default_answer=y", "-batch=on", "upgrade", package])
 
         if not module.check_mode and not query_package(module, slackpkg_path,
                                                        package):
@@ -174,7 +171,8 @@ def upgrade_packages(module, slackpkg_path, packages):
 
 
 def update_cache(module, slackpkg_path):
-    rc, out, err = module.run_command("%s -batch=on update" % (slackpkg_path))
+    rc, out, err = module.run_command(
+        [slackpkg_path, "-batch=on", "update"])
     if rc != 0:
         module.fail_json(msg="Could not update package cache")
 
diff --git a/plugins/modules/svr4pkg.py b/plugins/modules/svr4pkg.py
index db9902c770..56ded66e62 100644
--- a/plugins/modules/svr4pkg.py
+++ b/plugins/modules/svr4pkg.py
@@ -120,7 +120,7 @@ def package_installed(module, name, category):
     if category:
         cmd.append('-c')
     cmd.append(name)
-    rc, out, err = module.run_command(' '.join(cmd))
+    rc, out, err = module.run_command(cmd)
     if rc == 0:
         return True
     else:
diff --git a/plugins/modules/swdepot.py b/plugins/modules/swdepot.py
index 28a8ce3145..9ba1b02b30 100644
--- a/plugins/modules/swdepot.py
+++ b/plugins/modules/swdepot.py
@@ -68,7 +68,6 @@ EXAMPLES = '''
 import re
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six.moves import shlex_quote
 
 
 def compare_package(version1, version2):
@@ -94,13 +93,13 @@ def compare_package(version1, version2):
 def query_package(module, name, depot=None):
     """ Returns whether a package is installed or not and version. """
 
-    cmd_list = '/usr/sbin/swlist -a revision -l product'
+    cmd_list = ['/usr/sbin/swlist', '-a', 'revision', '-l', 'product']
     if depot:
-        rc, stdout, stderr = module.run_command("%s -s %s %s | grep %s" % (cmd_list, shlex_quote(depot), shlex_quote(name), shlex_quote(name)),
-                                                use_unsafe_shell=True)
-    else:
-        rc, stdout, stderr = module.run_command("%s %s | grep %s" % (cmd_list, shlex_quote(name), shlex_quote(name)), use_unsafe_shell=True)
+        cmd_list.extend(['-s', depot])
+    cmd_list.append(name)
+    rc, stdout, stderr = module.run_command(cmd_list)
     if rc == 0:
+        stdout = ''.join(line for line in stdout.splitlines(True) if name in line)
         version = re.sub(r"\s\s+|\t", " ", stdout).strip().split()[1]
     else:
         version = None
@@ -112,7 +111,7 @@ def remove_package(module, name):
     """ Uninstall package if installed. """
 
     cmd_remove = '/usr/sbin/swremove'
-    rc, stdout, stderr = module.run_command("%s %s" % (cmd_remove, name))
+    rc, stdout, stderr = module.run_command([cmd_remove, name])
 
     if rc == 0:
         return rc, stdout
@@ -123,8 +122,8 @@ def remove_package(module, name):
 def install_package(module, depot, name):
     """ Install package if not already installed """
 
-    cmd_install = '/usr/sbin/swinstall -x mount_all_filesystems=false'
-    rc, stdout, stderr = module.run_command("%s -s %s %s" % (cmd_install, depot, name))
+    cmd_install = ['/usr/sbin/swinstall', '-x', 'mount_all_filesystems=false']
+    rc, stdout, stderr = module.run_command(cmd_install + ["-s", depot, name])
     if rc == 0:
         return rc, stdout
     else: