swith to std::optional & remove not needed description.

Author: sashwinbalaji

src/traced/probes/ftrace/tracefs.cc

@@ -450,30 +450,30 @@ size_t Tracefs::NumberOfCpus() const {
   return num_cpus;
 }
 
-bool Tracefs::GetOfflineCpus(std::vector<uint32_t>* offline_cpus) {
+std::optional<std::vector<uint32_t>> Tracefs::GetOfflineCpus() {
   std::string offline_cpus_str;
   if (!ReadFile("/sys/devices/system/cpu/offline", &offline_cpus_str)) {
     PERFETTO_ELOG("Failed to read offline cpus file");
-    return false;
+    return std::nullopt;
   }
   offline_cpus_str = base::TrimWhitespace(offline_cpus_str);
 
   // The offline cpus file contains a list of comma-separated CPU ranges.
   // Each range is either a single CPU or a range of CPUs, e.g. "0-3,5,7-9".
   // Source: https://docs.kernel.org/admin-guide/cputopology.html
-  std::vector<uint32_t> offline_cpus_tmp;
+  std::vector<uint32_t> offline_cpus;
   for (base::StringSplitter ss(offline_cpus_str, ','); ss.Next();) {
     base::StringView offline_cpu_range(ss.cur_token(), ss.cur_token_size());
     size_t dash_pos = offline_cpu_range.find('-');
     if (dash_pos == base::StringView::npos) {
       // Single CPU in the format of "%d".
       std::optional<uint32_t> cpu = base::StringViewToUInt32(offline_cpu_range);
       if (cpu.has_value()) {
-        offline_cpus_tmp.push_back(cpu.value());
+        offline_cpus.push_back(cpu.value());
       } else {
         PERFETTO_ELOG("Failed to parse single CPU from offline CPU range: %s",
                       offline_cpu_range.data());
-        return false;
+        return std::nullopt;
       }
     } else {
       // Range of CPUs in the format of "%d-%d".
@@ -483,17 +483,16 @@ bool Tracefs::GetOfflineCpus(std::vector<uint32_t>* offline_cpus) {
           base::StringViewToUInt32(offline_cpu_range.substr(dash_pos + 1));
       if (start_cpu.has_value() && end_cpu.has_value()) {
         for (auto cpu = start_cpu.value(); cpu <= end_cpu.value(); ++cpu) {
-          offline_cpus_tmp.push_back(cpu);
+          offline_cpus.push_back(cpu);
         }
       } else {
         PERFETTO_ELOG("Failed to parse CPU range from offline CPU range: %s",
                       offline_cpu_range.data());
-        return false;
+        return std::nullopt;
       }
     }
   }
-  *offline_cpus = std::move(offline_cpus_tmp);
-  return true;
+  return offline_cpus;
 }
 
 void Tracefs::ClearTrace() {
@@ -506,17 +505,12 @@ void Tracefs::ClearTrace() {
   // In case some of the CPUs were not online, their buffer needs to be
   // cleared manually.
   //
-  // Note: There is a small time gap between when we clear the trace file and
-  // when we get the offline cpus. This introduces a small chance that some CPUs
-  // are missed if they came online in between those two points. This is a
-  // best-effort approach to keep the logic simple.
-  //
   // We cannot use PERFETTO_CHECK as we might get a permission denied error
   // on Android. The permissions to these files are configured in
   // platform/framework/native/cmds/atrace/atrace.rc.
-  std::vector<uint32_t> offline_cpus;
-  if (GetOfflineCpus(&offline_cpus)) {
-    for (const auto& cpu : offline_cpus) {
+  std::optional<std::vector<uint32_t>> offline_cpus = GetOfflineCpus();
+  if (offline_cpus.has_value()) {
+    for (const auto& cpu : offline_cpus.value()) {
       ClearPerCpuTrace(cpu);
     }
   } else {

src/traced/probes/ftrace/tracefs.h

@@ -18,6 +18,7 @@
 #define SRC_TRACED_PROBES_FTRACE_TRACEFS_H_
 
 #include <memory>
+#include <optional>
 #include <set>
 #include <string>
 #include <vector>
@@ -152,9 +153,10 @@ class Tracefs {
   // This will match the number of tracing/per_cpu/cpuXX directories.
   size_t virtual NumberOfCpus() const;
 
-  // Parses the list of offline CPUs from /sys/devices/system/cpu/offline into
-  // |offline_cpus|. Returns true on success.
-  bool GetOfflineCpus(std::vector<uint32_t>* offline_cpus);
+  // Parses the list of offline CPUs from "/sys/devices/system/cpu/offline" and
+  // returns them as a vector if successful, or std::nullopt if any failure in
+  // reading or parsing.
+  std::optional<std::vector<uint32_t>> GetOfflineCpus();
 
   // Clears the trace buffers for all CPUs. Blocks until this is done.
   void ClearTrace();

src/traced/probes/ftrace/tracefs_unittest.cc

@@ -27,6 +27,7 @@ using testing::AnyNumber;
 using testing::DoAll;
 using testing::ElementsAre;
 using testing::IsEmpty;
+using testing::Optional;
 using testing::Return;
 using testing::SetArgPointee;
 using testing::UnorderedElementsAre;
@@ -153,52 +154,36 @@ TEST(TracefsTest, ReadBufferSizeInPages) {
 
 TEST(TracefsTest, GetOfflineCpus) {
   MockTracefs ftrace;
-  std::vector<uint32_t> cpus;
 
   // ReadFile fails.
   EXPECT_CALL(ftrace, ReadFile("/sys/devices/system/cpu/offline", _))
       .WillOnce(Return(false));
-  EXPECT_FALSE(ftrace.Tracefs::GetOfflineCpus(&cpus));
-  EXPECT_THAT(cpus, IsEmpty());
-  testing::Mock::VerifyAndClearExpectations(&ftrace);
+  EXPECT_EQ(ftrace.Tracefs::GetOfflineCpus(), std::nullopt);
 
   // Invalid value.
   EXPECT_CALL(ftrace, ReadFile("/sys/devices/system/cpu/offline", _))
       .WillOnce(DoAll(SetArgPointee<1>("1,a,3"), Return(true)));
-  EXPECT_FALSE(ftrace.Tracefs::GetOfflineCpus(&cpus));
-  EXPECT_THAT(cpus, IsEmpty());
-  testing::Mock::VerifyAndClearExpectations(&ftrace);
+  EXPECT_EQ(ftrace.Tracefs::GetOfflineCpus(), std::nullopt);
 
   // Empty offline CPU list.
   EXPECT_CALL(ftrace, ReadFile("/sys/devices/system/cpu/offline", _))
       .WillOnce(DoAll(SetArgPointee<1>(""), Return(true)));
-  EXPECT_TRUE(ftrace.Tracefs::GetOfflineCpus(&cpus));
-  EXPECT_THAT(cpus, IsEmpty());
-  testing::Mock::VerifyAndClearExpectations(&ftrace);
+  EXPECT_THAT(ftrace.Tracefs::GetOfflineCpus(), Optional(IsEmpty()));
 
   // Comma-separated list of single offline CPUs.
   EXPECT_CALL(ftrace, ReadFile("/sys/devices/system/cpu/offline", _))
       .WillOnce(DoAll(SetArgPointee<1>("1,3\n"), Return(true)));
-  EXPECT_TRUE(ftrace.Tracefs::GetOfflineCpus(&cpus));
-  EXPECT_THAT(cpus, ElementsAre(1, 3));
-  cpus.clear();
-  testing::Mock::VerifyAndClearExpectations(&ftrace);
+  EXPECT_THAT(ftrace.GetOfflineCpus(), Optional(ElementsAre(1, 3)));
 
   // Range of offline CPUs (e.g., "0-2").
   EXPECT_CALL(ftrace, ReadFile("/sys/devices/system/cpu/offline", _))
       .WillOnce(DoAll(SetArgPointee<1>("0-2,4-5\n"), Return(true)));
-  EXPECT_TRUE(ftrace.Tracefs::GetOfflineCpus(&cpus));
-  EXPECT_THAT(cpus, ElementsAre(0, 1, 2, 4, 5));
-  cpus.clear();
-  testing::Mock::VerifyAndClearExpectations(&ftrace);
+  EXPECT_THAT(ftrace.GetOfflineCpus(), Optional(ElementsAre(0, 1, 2, 4, 5)));
 
   // Combination of single CPUs and ranges.
   EXPECT_CALL(ftrace, ReadFile("/sys/devices/system/cpu/offline", _))
       .WillOnce(DoAll(SetArgPointee<1>("0,2-3,5\n"), Return(true)));
-  EXPECT_TRUE(ftrace.Tracefs::GetOfflineCpus(&cpus));
-  EXPECT_THAT(cpus, ElementsAre(0, 2, 3, 5));
-  cpus.clear();
-  testing::Mock::VerifyAndClearExpectations(&ftrace);
+  EXPECT_THAT(ftrace.GetOfflineCpus(), Optional(ElementsAre(0, 2, 3, 5)));
 }
 
 }  // namespace