Improve const-correctness of dso_from_proc_line

DataDog · Oct 5, 2023 · 0906294 · 0906294
1 parent 5555d4b
commit 0906294
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 41 deletions.
diff --git a/include/dso_hdr.hpp b/include/dso_hdr.hpp
@@ -131,7 +131,7 @@ class DsoHdr {
   static DsoConstRange get_elf_range(const DsoMap &map, DsoMapConstIt it);
 
   // Helper to create a dso from a line in /proc/pid/maps
-  static Dso dso_from_proc_line(int pid, char *line);
+  static Dso dso_from_proc_line(int pid, const char *line);
 
   static DsoFindRes find_res_not_found(const DsoMap &map) {
     return {map.end(), false};

diff --git a/src/dso_hdr.cc b/src/dso_hdr.cc
@@ -469,7 +469,7 @@ bool DsoHdr::pid_backpopulate(PidMapping &pid_mapping, pid_t pid,
   return true;
 }
 
-Dso DsoHdr::dso_from_proc_line(int pid, char *line) {
+Dso DsoHdr::dso_from_proc_line(int pid, const char *line) {
   // clang-format off
   // Example of format
   /*
@@ -508,22 +508,20 @@ Dso DsoHdr::dso_from_proc_line(int pid, char *line) {
     return {};
   }
 
-  // Make sure the name index points to a valid char
-  char *p = &line[m_p];
-  char *q;
-  while (isspace(*p)) {
-    p++;
-  }
-  q = strchr(p, '\n');
-  if (q) {
-    *q = '\0';
+  // trim spaces on the left
+  std::string_view remaining{line + m_p};
+  remaining.remove_prefix(
+      +std::min(remaining.find_first_not_of(" \t"), remaining.size()));
+  // remove new line at end if present
+  if (remaining.ends_with('\n')) {
+    remaining.remove_suffix(1);
   }
 
   return {pid,
           m_start,
           m_end - 1,
           m_off,
-          std::string(p),
+          std::string(remaining),
           m_inode,
           mode_string_to_prot(m_mode),
           DsoOrigin::kProcMaps};

diff --git a/test/dso-ut.cc b/test/dso-ut.cc
@@ -231,92 +231,80 @@ static const char *const s_bad_line = "7b5242e44000-7b5242e45000 r-xp  00000000
 
 TEST(DSOTest, dso_from_proc_line) {
   LogHandle handle;
-  Dso no_exec =
-      DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_line_noexec));
+  Dso no_exec = DsoHdr::dso_from_proc_line(10, s_line_noexec);
   EXPECT_EQ(no_exec._type, DsoType::kStandard);
   EXPECT_EQ(no_exec._prot, PROT_READ);
   EXPECT_EQ(no_exec._pid, 10);
-  Dso standard_dso =
-      DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_exec_line));
+  Dso standard_dso = DsoHdr::dso_from_proc_line(10, s_exec_line);
   { // standard
     EXPECT_EQ(standard_dso._type, DsoType::kStandard);
   }
   { // vdso
-    Dso vdso_dso =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_vdso_lib));
+    Dso vdso_dso = DsoHdr::dso_from_proc_line(10, s_vdso_lib);
     EXPECT_EQ(vdso_dso._type, DsoType::kVdso);
   }
   { // stack
-    Dso stack_dso =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_stack_line));
+    Dso stack_dso = DsoHdr::dso_from_proc_line(10, s_stack_line);
     EXPECT_EQ(stack_dso._type, DsoType::kStack);
   }
   { // inode
-    Dso inode_dso =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_inode_line));
+    Dso inode_dso = DsoHdr::dso_from_proc_line(10, s_inode_line);
     EXPECT_EQ(inode_dso._type, DsoType::kAnon);
   }
   {
     // jsa
-    Dso jsa_dso = DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_jsa_line));
+    Dso jsa_dso = DsoHdr::dso_from_proc_line(10, s_jsa_line);
     EXPECT_EQ(jsa_dso._type, DsoType::kRuntime);
   }
   {
     // dotnet dll
-    Dso dll_dso =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_dotnet_line));
+    Dso dll_dso = DsoHdr::dso_from_proc_line(10, s_dotnet_line);
     EXPECT_EQ(dll_dso._type, DsoType::kRuntime);
   }
   DsoHdr dso_hdr;
   {
     // check that we don't overlap between lines that end on same byte
-    Dso standard_dso_2 =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_exec_line2));
+    Dso standard_dso_2 = DsoHdr::dso_from_proc_line(10, s_exec_line2);
     EXPECT_EQ(standard_dso._type, DsoType::kStandard);
     dso_hdr.insert_erase_overlap(std::move(standard_dso_2));
     dso_hdr.insert_erase_overlap(std::move(standard_dso));
     EXPECT_EQ(dso_hdr.get_nb_dso(), 2);
   }
   {
     // check that we erase everything if we have an overlap
-    Dso standard_dso_3 =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_exec_line3));
+    Dso standard_dso_3 = DsoHdr::dso_from_proc_line(10, s_exec_line3);
     EXPECT_EQ(standard_dso._type, DsoType::kStandard);
     dso_hdr.insert_erase_overlap(std::move(standard_dso_3));
     EXPECT_EQ(dso_hdr.get_nb_dso(), 1);
   }
   {
     // check that we still match element number 3
-    Dso standard_dso_4 =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_exec_line4));
+    Dso standard_dso_4 = DsoHdr::dso_from_proc_line(10, s_exec_line4);
     ProcessAddress_t end_4 = standard_dso_4._end;
     DsoFindRes findres =
         dso_hdr.insert_erase_overlap(std::move(standard_dso_4));
     EXPECT_EQ(findres.first->second._end, end_4);
   }
   {
-    Dso dd_profiling_dso =
-        DsoHdr::dso_from_proc_line(10, const_cast<char *>(s_dd_profiling));
+    Dso dd_profiling_dso = DsoHdr::dso_from_proc_line(10, s_dd_profiling);
     EXPECT_EQ(dd_profiling_dso._type, DsoType::kDDProfiling);
   }
   {
-    Dso jitdump_dso =
-        DsoHdr::dso_from_proc_line(3237589, const_cast<char *>(s_jitdump_line));
+    Dso jitdump_dso = DsoHdr::dso_from_proc_line(3237589, s_jitdump_line);
     EXPECT_EQ(jitdump_dso._type, DsoType::kJITDump);
   }
   { // jitdump with a name different from PID (for whole host)
-    Dso jitdump_dso =
-        DsoHdr::dso_from_proc_line(12, const_cast<char *>(s_jitdump_line));
+    Dso jitdump_dso = DsoHdr::dso_from_proc_line(12, s_jitdump_line);
     EXPECT_EQ(jitdump_dso._type, DsoType::kJITDump);
   }
   {
     // empty file proc line
-    Dso dso = DsoHdr::dso_from_proc_line(12, const_cast<char *>(s_empty_file_line));
+    Dso dso = DsoHdr::dso_from_proc_line(12, s_empty_file_line);
     EXPECT_EQ(dso._type, DsoType::kAnon);
   }
   {
     // bad proc line
-    Dso dso = DsoHdr::dso_from_proc_line(12, const_cast<char *>(s_bad_line));
+    Dso dso = DsoHdr::dso_from_proc_line(12, s_bad_line);
     EXPECT_EQ(dso._pid, -1);
   }
 }
@@ -404,8 +392,7 @@ TEST(DSOTest, insert_jitdump) {
   DsoHdr dso_hdr;
   // pid from dso line (important for the jitdump name)
   pid_t test_pid = 3237589;
-  Dso jitdump_dso =
-      DsoHdr::dso_from_proc_line(test_pid, const_cast<char *>(s_jitdump_line));
+  Dso jitdump_dso = DsoHdr::dso_from_proc_line(test_pid, s_jitdump_line);
   EXPECT_EQ(jitdump_dso._type, DsoType::kJITDump);
   ProcessAddress_t start = jitdump_dso._start;
   DsoHdr::PidMapping &pid_mapping = dso_hdr._pid_map[test_pid];