Merge pull request openSUSE#841 from aschnell/master

- make SystemCmd take vector of strings
aschnell · Oct 13, 2023 · 55a1731 · 55a1731
2 parents 4e765cc + cba58ae
commit 55a1731
Show file tree

Hide file tree

Showing 12 changed files with 180 additions and 88 deletions.
diff --git a/LIBVERSION b/LIBVERSION
@@ -1 +1 @@
-7.2.0
+7.3.0
diff --git a/client/misc.cc b/client/misc.cc
@@ -205,7 +205,7 @@ Differ::run(const string& f1, const string& f2) const
 	tmp += " " + extensions;
     tmp += " " + quote(f1) + " " + quote(f2);
 
-    SystemCmd cmd(tmp);
+    SystemCmd cmd({ SH_BIN, "-c", tmp });
 
     for (const string& line : cmd.get_stdout())
 	cout << line << endl;

diff --git a/package/snapper.changes b/package/snapper.changes
@@ -2,6 +2,7 @@
 Fri Oct 13 08:56:18 CEST 2023 - aschnell@suse.com
 
 - fix diff for lvm based configs (bsc#1216191)
+- make SystemCmd take vector of strings
 
 -------------------------------------------------------------------
 Thu Sep 14 15:45:53 CEST 2023 - aschnell@suse.com

diff --git a/snapper/Ext4.cc b/snapper/Ext4.cc
@@ -91,7 +91,7 @@ namespace snapper
 	int r1 = mkdir((subvolume + "/.snapshots").c_str(), 0700);
 	if (r1 == 0)
 	{
-	    SystemCmd cmd1(CHATTRBIN " +x " + quote(subvolume + "/.snapshots"));
+	    SystemCmd cmd1({ CHATTRBIN, "+x", subvolume + "/.snapshots" });
 	    if (cmd1.retcode() != 0)
 		throw CreateConfigFailedException("chattr failed");
 	}
@@ -104,7 +104,7 @@ namespace snapper
 	int r2 = mkdir((subvolume + "/.snapshots/.info").c_str(), 0700);
 	if (r2 == 0)
 	{
-	    SystemCmd cmd2(CHATTRBIN " -x " + quote(subvolume + "/.snapshots/.info"));
+	    SystemCmd cmd2({ CHATTRBIN, "-x", subvolume + "/.snapshots/.info" });
 	    if (cmd2.retcode() != 0)
 		throw CreateConfigFailedException("chattr failed");
 	}
@@ -174,11 +174,11 @@ namespace snapper
 	if (num_parent != 0 || !read_only)
 	    throw std::logic_error("not implemented");
 
-	SystemCmd cmd1(TOUCHBIN " " + quote(snapshotFile(num)));
+	SystemCmd cmd1({ TOUCHBIN, snapshotFile(num) });
 	if (cmd1.retcode() != 0)
 	    throw CreateSnapshotFailedException();
 
-	SystemCmd cmd2(CHSNAPBIN " +S " + quote(snapshotFile(num)));
+	SystemCmd cmd2({ CHSNAPBIN, "+S", snapshotFile(num) });
 	if (cmd2.retcode() != 0)
 	    throw CreateSnapshotFailedException();
     }
@@ -187,7 +187,7 @@ namespace snapper
     void
     Ext4::deleteSnapshot(unsigned int num) const
     {
-	SystemCmd cmd(CHSNAPBIN " -S " + quote(snapshotFile(num)));
+	SystemCmd cmd({ CHSNAPBIN, "-S", snapshotFile(num) });
 	if (cmd.retcode() != 0)
 	    throw DeleteSnapshotFailedException();
     }
@@ -212,7 +212,7 @@ namespace snapper
 	if (isSnapshotMounted(num))
 	    return;
 
-	SystemCmd cmd1(CHSNAPBIN " +n " + quote(snapshotFile(num)));
+	SystemCmd cmd1({ CHSNAPBIN, "+n", snapshotFile(num) });
 	if (cmd1.retcode() != 0)
 	    throw MountSnapshotFailedException();
 
@@ -237,7 +237,7 @@ namespace snapper
 	// if (!umount(snapshotDir(num)))
 	// throw UmountSnapshotFailedException();
 
-	SystemCmd cmd1(CHSNAPBIN " -n " + quote(snapshotFile(num)));
+	SystemCmd cmd1({ CHSNAPBIN, "-n", snapshotFile(num) });
 	if (cmd1.retcode() != 0)
 	    throw UmountSnapshotFailedException();
 

diff --git a/snapper/Hooks.cc b/snapper/Hooks.cc
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) [2011-2015] Novell, Inc.
- * Copyright (c) 2022 SUSE LLC
+ * Copyright (c) [2022-2023] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -53,10 +53,9 @@ namespace snapper
 	    std::sort(scripts.begin(), scripts.end());
 	    for (const string& script : scripts)
 	    {
-		string cmd_line = dir.fullname(script);
-		for (const string& arg : args)
-		    cmd_line += " " + quote(arg);
-		SystemCmd cmd(cmd_line);
+		SystemCmd::Args cmd_args = { dir.fullname(script) };
+		cmd_args << args;
+		SystemCmd cmd(cmd_args);
 	    }
 	}
 	catch (const Exception& e)
@@ -185,7 +184,7 @@ namespace snapper
 
 	if (subvolume == "/" && filesystem->fstype() == "btrfs" && access(GRUB_SCRIPT, X_OK) == 0)
 	{
-	    SystemCmd cmd(string(GRUB_SCRIPT) + " " + option);
+	    SystemCmd cmd({ GRUB_SCRIPT, option });
 	}
 #endif
     }
@@ -201,7 +200,7 @@ namespace snapper
 	// Fate#319108
 	if (access(ROLLBACK_SCRIPT, X_OK) == 0)
 	{
-	    SystemCmd cmd(string(ROLLBACK_SCRIPT) + " " + old_root + " " + new_root);
+	    SystemCmd cmd({ ROLLBACK_SCRIPT, old_root, new_root });
 	}
 #endif
     }

diff --git a/snapper/Lvm.cc b/snapper/Lvm.cc
@@ -483,7 +483,7 @@ namespace snapper
 
     LvmCapabilities::LvmCapabilities()
     {
-	SystemCmd cmd(string(LVMBIN " version"));
+	SystemCmd cmd({ LVMBIN, "version" });
 
 	if (cmd.retcode() != 0 || cmd.get_stdout().empty())
 	{
@@ -510,7 +510,7 @@ namespace snapper
 		lvm_version version(maj, min, rev);
 
 		if (version >= lvm_version(2, 2, 99))
-		    ignoreactivationskip = " -K";
+		    ignoreactivationskip = "--ignoreactivationskip";
 	    }
 	}
     }

diff --git a/snapper/Lvm.h b/snapper/Lvm.h
@@ -72,7 +72,7 @@ namespace snapper
 
 	LvmCapabilities();
 
-	// empty or " -K" if lvm supports ignore activation skip flag
+	// empty or "--ignoreactivationskip" if lvm supports ignore activation skip flag
 	string ignoreactivationskip;
 
     };

diff --git a/snapper/LvmCache.cc b/snapper/LvmCache.cc
@@ -91,7 +91,12 @@ namespace snapper
 	{
 	    boost::upgrade_to_unique_lock<boost::shared_mutex> unique_lock(upg_lock);
 
-	    SystemCmd cmd(LVCHANGEBIN + caps->get_ignoreactivationskip() + " -ay " + quote(full_name()));
+	    SystemCmd::Args cmd_args = { LVCHANGEBIN };
+	    if (!caps->get_ignoreactivationskip().empty())
+		cmd_args << caps->get_ignoreactivationskip();
+	    cmd_args << "--activate" << "y" << full_name();
+
+	    SystemCmd cmd(cmd_args);
 	    if (cmd.retcode() != 0)
 	    {
 		y2err("lvm cache: " << full_name() << " activation failed!");
@@ -120,7 +125,7 @@ namespace snapper
 	{
 	    boost::upgrade_to_unique_lock<boost::shared_mutex> unique_lock(upg_lock);
 
-	    SystemCmd cmd(LVCHANGEBIN " -an " + quote(full_name()));
+	    SystemCmd cmd({ LVCHANGEBIN, "--activate", "n", full_name() });
 	    if (cmd.retcode() != 0)
 	    {
 		y2err("lvm cache: " << full_name() << " deactivation failed!");
@@ -139,7 +144,7 @@ namespace snapper
     {
 	boost::unique_lock<boost::shared_mutex> unique_lock(lv_mutex);
 
-	SystemCmd cmd(LVSBIN " --noheadings -o lv_attr,segtype " + quote(full_name()));
+	SystemCmd cmd({ LVSBIN, "--noheadings", "--options", "lv_attr,segtype", full_name() });
 	if (cmd.retcode() != 0 || cmd.get_stdout().empty())
 	{
 	    y2err("lvm cache: failed to get info about " << full_name());
@@ -181,8 +186,7 @@ namespace snapper
 	{
 	    boost::upgrade_to_unique_lock<boost::shared_mutex> unique_lock(upg_lock);
 
-	    SystemCmd cmd(LVCHANGEBIN " --permission " + string(read_only ? "r" : "rw") + " " +
-			  quote(full_name()));
+	    SystemCmd cmd({ LVCHANGEBIN, "--permission", read_only ? "r" : "rw", full_name() });
 	    if (cmd.retcode() != 0)
 	    {
 		y2err("lvm cache: " << full_name() << " setting permission failed!");
@@ -335,8 +339,8 @@ namespace snapper
 
 	boost::upgrade_to_unique_lock<boost::shared_mutex> unique_lock(upg_lock);
 
-	SystemCmd cmd(LVCREATEBIN " --permission " + string(read_only ? "r" : "rw") + " --snapshot "
-		      "--name " + quote(lv_snapshot_name) + " " + quote(full_name(lv_origin_name)));
+	SystemCmd cmd({ LVCREATEBIN, "--permission", read_only ? "r" : "rw", "--snapshot",
+		"--name", lv_snapshot_name, full_name(lv_origin_name) });
 
 	if (cmd.retcode() != 0)
 	    throw LvmCacheException();
@@ -359,7 +363,7 @@ namespace snapper
 	}
 	else
 	{
-	    SystemCmd cmd(LVSBIN " --noheadings -o lv_attr,segtype " + quote(full_name(lv_name)));
+	    SystemCmd cmd({ LVSBIN, "--noheadings", "--options", "lv_attr,segtype", full_name(lv_name) });
 	    if (cmd.retcode() != 0 || cmd.get_stdout().empty())
 	    {
 		y2err("lvm cache: failed to get info about " << full_name(lv_name));
@@ -395,7 +399,7 @@ namespace snapper
 	// wait for all invidual lv cache operations under shared vg lock to finish
 	boost::upgrade_to_unique_lock<boost::shared_mutex> unique_lock(upg_lock);
 
-	SystemCmd cmd(LVREMOVEBIN " --force " + quote(full_name(lv_name)));
+	SystemCmd cmd({ LVREMOVEBIN, "--force", full_name(lv_name) });
 	if (cmd.retcode() != 0)
 	    throw LvmCacheException();
 
@@ -548,7 +552,7 @@ namespace snapper
     void
     LvmCache::add_vg(const string& vg_name, const string& include_lv_name)
     {
-	SystemCmd cmd(LVSBIN " --noheadings -o lv_name,lv_attr,segtype " + quote(vg_name));
+	SystemCmd cmd({ LVSBIN, "--noheadings", "--options", "lv_name,lv_attr,segtype", vg_name });
 	if (cmd.retcode() != 0)
 	{
 	    y2err("lvm cache: failed to get info about VG " << vg_name);

diff --git a/snapper/Snapper.cc b/snapper/Snapper.cc
@@ -453,7 +453,7 @@ namespace snapper
 
 	    sysconfig.save();
 
-	    SystemCmd cmd(RMBIN " " + quote(CONFIGS_DIR "/" + config_name));
+	    SystemCmd cmd({ RMBIN, "--", CONFIGS_DIR "/" + config_name });
 
 	    SN_RETHROW(e);
 	}
@@ -512,7 +512,7 @@ namespace snapper
 	    SN_THROW(DeleteConfigFailedException("deleting snapshot failed"));
 	}
 
-	SystemCmd cmd1(RMBIN " " + quote(CONFIGS_DIR "/" + config_name));
+	SystemCmd cmd1({ RMBIN, "--", CONFIGS_DIR "/" + config_name });
 	if (cmd1.retcode() != 0)
 	{
 	    SN_THROW(DeleteConfigFailedException("deleting config-file failed"));