update_engine: Remove usage of g_shell_parse_argv().
g_shell_parse_argv() was used as a helper function to specify a command
line in unittests as a string and convert it to a list of command
arguments. With C++11, the overhead of declaring them as a vector of
arguments is minimal, so we just remove the usage of the function
g_shell_parse_argv() completely.
BUG=chromium:499886
TEST=uniitest still pass.
Change-Id: Ibb63ea2cc85d6976bfd45c496906bfcce1f421d6
Reviewed-on: https://chromium-review.googlesource.com/284890
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
diff --git a/fake_p2p_manager_configuration.h b/fake_p2p_manager_configuration.h
index 6272689..1dc7c6e 100644
--- a/fake_p2p_manager_configuration.h
+++ b/fake_p2p_manager_configuration.h
@@ -12,11 +12,9 @@
#include <string>
#include <vector>
-#include <glib.h>
-
#include <base/logging.h>
#include <base/strings/string_util.h>
-#include <base/strings/stringprintf.h>
+#include <base/strings/string_number_conversions.h>
namespace chromeos_update_engine {
@@ -24,12 +22,8 @@
// /var/cache/p2p, a temporary directory is used.
class FakeP2PManagerConfiguration : public P2PManager::Configuration {
public:
- FakeP2PManagerConfiguration()
- : p2p_client_cmdline_format_(
- "p2p-client --get-url={file_id} --minimum-size={minsize}") {
+ FakeP2PManagerConfiguration() {
EXPECT_TRUE(utils::MakeTempDirectory("/tmp/p2p-tc.XXXXXX", &p2p_dir_));
- SetInitctlStartCommandLine("initctl start p2p");
- SetInitctlStopCommandLine("initctl stop p2p");
}
~FakeP2PManagerConfiguration() {
@@ -51,76 +45,53 @@
// P2PManager::Configuration override
std::vector<std::string> GetP2PClientArgs(const std::string &file_id,
size_t minimum_size) override {
- std::string formatted_command_line = p2p_client_cmdline_format_;
+ std::vector<std::string> formatted_command = p2p_client_cmd_format_;
// Replace {variable} on the passed string.
- ReplaceSubstringsAfterOffset(
- &formatted_command_line, 0, "{file_id}", file_id);
- ReplaceSubstringsAfterOffset(
- &formatted_command_line, 0,
- "{minsize}", base::StringPrintf("%zu", minimum_size));
-
- return ParseCommandLine(formatted_command_line);
+ std::string str_minimum_size = base::SizeTToString(minimum_size);
+ for (std::string& arg : formatted_command) {
+ ReplaceSubstringsAfterOffset(&arg, 0, "{file_id}", file_id);
+ ReplaceSubstringsAfterOffset(&arg, 0, "{minsize}", str_minimum_size);
+ }
+ return formatted_command;
}
// Use |command_line| instead of "initctl start p2p" when attempting
// to start the p2p service.
- void SetInitctlStartCommandLine(const std::string &command_line) {
- initctl_start_args_ = ParseCommandLine(command_line);
+ void SetInitctlStartCommand(const std::vector<std::string>& command) {
+ initctl_start_args_ = command;
}
// Use |command_line| instead of "initctl stop p2p" when attempting
// to stop the p2p service.
- void SetInitctlStopCommandLine(const std::string &command_line) {
- initctl_stop_args_ = ParseCommandLine(command_line);
+ void SetInitctlStopCommand(const std::vector<std::string>& command) {
+ initctl_stop_args_ = command;
}
- // Use |command_line_format| instead of "p2p-client --get-url={file_id}
+ // Use |command_format| instead of "p2p-client --get-url={file_id}
// --minimum-size={minsize}" when attempting to look up a file using
// p2p-client(1).
//
- // The passed |command_line_format| argument can have "{file_id}" and
- // "{minsize}" as substrings, that will be replaced by the corresponding
- // values passed to GetP2PClientArgs().
- void SetP2PClientCommandLine(const std::string &command_line_format) {
- p2p_client_cmdline_format_ = command_line_format;
+ // The passed |command_format| argument can have "{file_id}" and "{minsize}"
+ // as substrings of any of its elements, that will be replaced by the
+ // corresponding values passed to GetP2PClientArgs().
+ void SetP2PClientCommand(const std::vector<std::string>& command_format) {
+ p2p_client_cmd_format_ = command_format;
}
private:
- // Helper for parsing and splitting |command_line| into an argument
- // vector in much the same way a shell would except for not
- // supporting wildcards, globs, operators etc. See
- // g_shell_parse_argv() for more details. If an error occurs, the
- // empty vector is returned.
- std::vector<std::string> ParseCommandLine(const std::string &command_line) {
- gint argc;
- gchar **argv;
- std::vector<std::string> ret;
-
- if (!g_shell_parse_argv(command_line.c_str(),
- &argc,
- &argv,
- nullptr)) {
- LOG(ERROR) << "Error splitting '" << command_line << "'";
- return ret;
- }
- for (int n = 0; n < argc; n++)
- ret.push_back(argv[n]);
- g_strfreev(argv);
- return ret;
- }
-
// The temporary directory used for p2p.
std::string p2p_dir_;
// Argument vector for starting p2p.
- std::vector<std::string> initctl_start_args_;
+ std::vector<std::string> initctl_start_args_{"initctl", "start", "p2p"};
// Argument vector for stopping p2p.
- std::vector<std::string> initctl_stop_args_;
+ std::vector<std::string> initctl_stop_args_{"initctl", "stop", "p2p"};
// A string for generating the p2p-client command. See the
// SetP2PClientCommandLine() for details.
- std::string p2p_client_cmdline_format_;
+ std::vector<std::string> p2p_client_cmd_format_{
+ "p2p-client", "--get-url={file_id}", "--minimum-size={minsize}"};
DISALLOW_COPY_AND_ASSIGN(FakeP2PManagerConfiguration);
};
diff --git a/p2p_manager_unittest.cc b/p2p_manager_unittest.cc
index 7c4cfd9..3926f93 100644
--- a/p2p_manager_unittest.cc
+++ b/p2p_manager_unittest.cc
@@ -12,6 +12,8 @@
#include <attr/xattr.h> // NOLINT - requires typed defined in unistd.h
#include <memory>
+#include <string>
+#include <vector>
#include <base/bind.h>
#include <base/callback.h>
@@ -36,6 +38,7 @@
using chromeos::MessageLoop;
using chromeos_update_engine::test_utils::System;
using std::string;
+using std::vector;
using std::unique_ptr;
using testing::DoAll;
using testing::Return;
@@ -454,30 +457,30 @@
// behaviours of initctl(8) that we rely on.
TEST_F(P2PManagerTest, StartP2P) {
// Check that we can start the service
- test_conf_->SetInitctlStartCommandLine("true");
+ test_conf_->SetInitctlStartCommand({"true"});
EXPECT_TRUE(manager_->EnsureP2PRunning());
- test_conf_->SetInitctlStartCommandLine("false");
+ test_conf_->SetInitctlStartCommand({"false"});
EXPECT_FALSE(manager_->EnsureP2PRunning());
- test_conf_->SetInitctlStartCommandLine(
- "sh -c 'echo \"initctl: Job is already running: p2p\" >&2; false'");
+ test_conf_->SetInitctlStartCommand({
+ "sh", "-c", "echo \"initctl: Job is already running: p2p\" >&2; false"});
EXPECT_TRUE(manager_->EnsureP2PRunning());
- test_conf_->SetInitctlStartCommandLine(
- "sh -c 'echo something else >&2; false'");
+ test_conf_->SetInitctlStartCommand({
+ "sh", "-c", "echo something else >&2; false"});
EXPECT_FALSE(manager_->EnsureP2PRunning());
}
// Same comment as for StartP2P
TEST_F(P2PManagerTest, StopP2P) {
// Check that we can start the service
- test_conf_->SetInitctlStopCommandLine("true");
+ test_conf_->SetInitctlStopCommand({"true"});
EXPECT_TRUE(manager_->EnsureP2PNotRunning());
- test_conf_->SetInitctlStopCommandLine("false");
+ test_conf_->SetInitctlStopCommand({"false"});
EXPECT_FALSE(manager_->EnsureP2PNotRunning());
- test_conf_->SetInitctlStopCommandLine(
- "sh -c 'echo \"initctl: Unknown instance \" >&2; false'");
+ test_conf_->SetInitctlStopCommand({
+ "sh", "-c", "echo \"initctl: Unknown instance \" >&2; false"});
EXPECT_TRUE(manager_->EnsureP2PNotRunning());
- test_conf_->SetInitctlStopCommandLine(
- "sh -c 'echo something else >&2; false'");
+ test_conf_->SetInitctlStopCommand({
+ "sh", "-c", "echo something else >&2; false"});
EXPECT_FALSE(manager_->EnsureP2PNotRunning());
}
@@ -492,40 +495,41 @@
TEST_F(P2PManagerTest, LookupURL) {
// Emulate p2p-client returning valid URL with "fooX", 42 and "cros_au"
// being propagated in the right places.
- test_conf_->SetP2PClientCommandLine(
- "echo 'http://1.2.3.4/{file_id}_{minsize}'");
+ test_conf_->SetP2PClientCommand({
+ "echo", "http://1.2.3.4/{file_id}_{minsize}"});
manager_->LookupUrlForFile("fooX", 42, TimeDelta(),
base::Bind(ExpectUrl,
"http://1.2.3.4/fooX.cros_au_42"));
loop_.Run();
// Emulate p2p-client returning invalid URL.
- test_conf_->SetP2PClientCommandLine("echo 'not_a_valid_url'");
+ test_conf_->SetP2PClientCommand({"echo", "not_a_valid_url"});
manager_->LookupUrlForFile("foobar", 42, TimeDelta(),
base::Bind(ExpectUrl, ""));
loop_.Run();
// Emulate p2p-client conveying failure.
- test_conf_->SetP2PClientCommandLine("false");
+ test_conf_->SetP2PClientCommand({"false"});
manager_->LookupUrlForFile("foobar", 42, TimeDelta(),
base::Bind(ExpectUrl, ""));
loop_.Run();
// Emulate p2p-client not existing.
- test_conf_->SetP2PClientCommandLine("/path/to/non/existent/helper/program");
+ test_conf_->SetP2PClientCommand({"/path/to/non/existent/helper/program"});
manager_->LookupUrlForFile("foobar", 42,
TimeDelta(),
base::Bind(ExpectUrl, ""));
loop_.Run();
// Emulate p2p-client crashing.
- test_conf_->SetP2PClientCommandLine("sh -c 'kill -SEGV $$'");
+ test_conf_->SetP2PClientCommand({"sh", "-c", "kill -SEGV $$"});
manager_->LookupUrlForFile("foobar", 42, TimeDelta(),
base::Bind(ExpectUrl, ""));
loop_.Run();
// Emulate p2p-client exceeding its timeout.
- test_conf_->SetP2PClientCommandLine("sh -c 'echo http://1.2.3.4/; sleep 2'");
+ test_conf_->SetP2PClientCommand({
+ "sh", "-c", "echo http://1.2.3.4/; sleep 2"});
manager_->LookupUrlForFile("foobar", 42, TimeDelta::FromMilliseconds(500),
base::Bind(ExpectUrl, ""));
loop_.Run();