perfetto_cmd: don't connect to the service if unnecessary
The previous refactoring aosp/1713963 introduced a bug
in the perfetto cmdline client. The bug was about assuming
that a 0 return code from the cmdline parser meant
"all clear, proceed with the service connection".
However, there were two cases where previously a return 0
mean to exit(0) without connecting:
--version and --reset-guardrails.
The CL makes the ParseCmdlineAndMaybeDaemonize return
an Optional<int> so it's clear if intends to exit()
or continue.
Test: added a test to perfetto_integrationtests,
checked it failed before this CL.
Bug: 189359657
Bug: 158465724
Bug: 187945217
Change-Id: I765dd36dc9b69576ec3e888dfa2893ea31a7b6e5diff --git a/src/perfetto_cmd/perfetto_cmd.cc b/src/perfetto_cmd/perfetto_cmd.cc
index c6fe500..6bca082 100644
--- a/src/perfetto_cmd/perfetto_cmd.cc
+++ b/src/perfetto_cmd/perfetto_cmd.cc
@@ -176,7 +176,7 @@
g_perfetto_cmd = nullptr;
}
-int PerfettoCmd::PrintUsage(const char* argv0) {
+void PerfettoCmd::PrintUsage(const char* argv0) {
fprintf(stderr, R"(
Usage: %s
--background -d : Exits immediately and continues in the background.
@@ -225,10 +225,10 @@
Exit code: 0:Yes, 2:No, 1:Error.
)", /* this comment fixes syntax highlighting in some editors */
argv0);
- return 1;
}
-int PerfettoCmd::ParseCmdlineAndMaybeDaemonize(int argc, char** argv) {
+base::Optional<int> PerfettoCmd::ParseCmdlineAndMaybeDaemonize(int argc,
+ char** argv) {
#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
umask(0000); // make sure that file creation is not affected by umask.
#endif
@@ -459,7 +459,8 @@
continue;
}
- return PrintUsage(argv[0]);
+ PrintUsage(argv[0]);
+ return 1;
}
for (ssize_t i = optind; i < argc; i++) {
@@ -693,7 +694,7 @@
base::Daemonize();
}
- return 0; // Continues in ConnectToServiceAndRun() below.
+ return base::nullopt; // Continues in ConnectToServiceAndRun() below.
}
int PerfettoCmd::ConnectToServiceAndRun() {
@@ -1095,9 +1096,9 @@
int PERFETTO_EXPORT_ENTRYPOINT PerfettoCmdMain(int argc, char** argv) {
perfetto::PerfettoCmd cmd;
- int res = cmd.ParseCmdlineAndMaybeDaemonize(argc, argv);
- if (res)
- return res;
+ auto opt_res = cmd.ParseCmdlineAndMaybeDaemonize(argc, argv);
+ if (opt_res.has_value())
+ return *opt_res;
return cmd.ConnectToServiceAndRun();
}
diff --git a/src/perfetto_cmd/perfetto_cmd.h b/src/perfetto_cmd/perfetto_cmd.h
index 4087f49..f1bfe5b 100644
--- a/src/perfetto_cmd/perfetto_cmd.h
+++ b/src/perfetto_cmd/perfetto_cmd.h
@@ -50,7 +50,10 @@
// The main() is split in two stages: cmdline parsing and actual interaction
// with traced. This is to allow tools like tracebox to avoid spawning the
// service for no reason if the cmdline parsing fails.
- int ParseCmdlineAndMaybeDaemonize(int argc, char** argv);
+ // Return value:
+ // nullopt: no error, the caller should call ConnectToServiceAndRun.
+ // 0-N: the caller should exit() with the given exit code.
+ base::Optional<int> ParseCmdlineAndMaybeDaemonize(int argc, char** argv);
int ConnectToServiceAndRun();
// perfetto::Consumer implementation.
@@ -69,7 +72,7 @@
bool OpenOutputFile();
void SetupCtrlCSignalHandler();
void FinalizeTraceAndExit();
- int PrintUsage(const char* argv0);
+ void PrintUsage(const char* argv0);
void PrintServiceState(bool success, const TracingServiceState&);
void OnTimeout();
bool is_detach() const { return !detach_key_.empty(); }
diff --git a/src/tracebox/tracebox.cc b/src/tracebox/tracebox.cc
index f99aaa8..ab2cae8 100644
--- a/src/tracebox/tracebox.cc
+++ b/src/tracebox/tracebox.cc
@@ -125,9 +125,9 @@
// spawned by the damonized cmdline client, which is what we want so killing
// the backgrounded cmdline client will also kill the other services, as they
// will live in the same background session.
- int res = perfetto_cmd.ParseCmdlineAndMaybeDaemonize(argc, argv);
- if (res)
- return res;
+ auto opt_res = perfetto_cmd.ParseCmdlineAndMaybeDaemonize(argc, argv);
+ if (opt_res.has_value())
+ return *opt_res;
std::string self_path = base::GetCurExecutablePath();
base::Subprocess traced({self_path, "traced"});
diff --git a/test/end_to_end_integrationtest.cc b/test/end_to_end_integrationtest.cc
index af54f08..435690d 100644
--- a/test/end_to_end_integrationtest.cc
+++ b/test/end_to_end_integrationtest.cc
@@ -1176,6 +1176,11 @@
EXPECT_THAT(stderr_, HasSubstr("Cannot specify a trace config"));
}
+TEST_F(PerfettoCmdlineTest, NoSanitizers(Version)) {
+ auto perfetto = ExecPerfetto({"--version"});
+ EXPECT_EQ(0, perfetto.Run(&stderr_)) << stderr_;
+}
+
TEST_F(PerfettoCmdlineTest, NoSanitizers(TxtConfig)) {
std::string cfg("duration_ms: 100");
auto perfetto = ExecPerfetto({"-c", "-", "--txt", "-o", "-"}, cfg);
