Overhaul base/process_util_linux.cc.

base/process_util_linux.cc

Index: base/process_util_linux.cc
===================================================================
--- base/process_util_linux.cc	(revision 125518)
+++ base/process_util_linux.cc	(working copy)
@@ -4,15 +4,10 @@
 
 #include "base/process_util.h"
 
-#include <ctype.h>
 #include <dirent.h>
-#include <dlfcn.h>
-#include <errno.h>
-#include <fcntl.h>
+#include <malloc.h>
 #include <sys/time.h>
 #include <sys/types.h>
-#include <sys/wait.h>
-#include <time.h>
 #include <unistd.h>
 
 #include "base/file_util.h"
@@ -32,26 +27,125 @@
 };
 
 const char kProcDir[] = "/proc";
+const char kStatFile[] = "stat";
 
 // Returns a FilePath to "/proc/pid".
 FilePath GetProcPidDir(pid_t pid) {
-  return FilePath(kProcDir).Append(base::Int64ToString(pid));
+  return FilePath(kProcDir).Append(base::IntToString(pid));
 }
 
-// Reads /proc/<pid>/stat and populates |proc_stats| with the values split by
-// spaces. Returns true if successful.
-bool GetProcStats(pid_t pid, std::vector<std::string>* proc_stats) {
+// Fields from /proc/<pid>/stat, 0-based. See man 5 proc.
+// If the ordering ever changes, carefully review functions that use these
+// values.
+enum ProcStatsFields {
+  VM_COMM           = 1,   // Filename of executable, without parentheses.
+  VM_STATE          = 2,   // Letter indicating the state of the process.
+  VM_PPID           = 3,   // PID of the parent.
+  VM_PGRP           = 4,   // Process group id.
+  VM_UTIME          = 13,  // Time scheduled in user mode in clock ticks.
+  VM_STIME          = 14,  // Time scheduled in kernel mode in clock ticks.
+  VM_VSIZE          = 22,  // Virtual memory size in bytes.
+  VM_RSS            = 23,  // Resident Set Size in pages.
+};
+
+// Reads /proc/<pid>/stat into |buffer|. Returns true if successful.
+bool ReadProcStats(pid_t pid, std::string* buffer) {
   // Synchronously reading files in /proc is safe.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
 
-  FilePath stat_file = GetProcPidDir(pid).Append("stat");
-  std::string mem_stats;
-  if (!file_util::ReadFileToString(stat_file, &mem_stats))
+  FilePath stat_file = GetProcPidDir(pid).Append(kStatFile);
+  if (!file_util::ReadFileToString(stat_file, buffer)) {
+    DLOG(WARNING) << "Failed to get process stats.";
     return false;
-  base::SplitString(mem_stats, ' ', proc_stats);
+  }
   return true;
 }
 
+// Takes |stats_data| and populates |proc_stats| with the values split by
+// spaces. Taking into account the 2nd field may, in itself, contain spaces.
+// Returns true if successful.
+bool ParseProcStats(const std::string& stats_data,
+                    std::vector<std::string>* proc_stats) {
+  // The stat file is formatted as:
+  // pid (process name) data1 data2 .... dataN
+  // Look for the closing paren by scanning backwards, to avoid being fooled by
+  // processes with ')' in the name.
+  size_t open_parens_idx = stats_data.find(" (");
+  size_t close_parens_idx = stats_data.rfind(") ");
+  if (open_parens_idx == std::string::npos ||
+      close_parens_idx == std::string::npos ||
+      open_parens_idx > close_parens_idx) {
+    NOTREACHED();
+    return false;
+  }
+  open_parens_idx++;
+
+  proc_stats->clear();
+  // PID.
+  proc_stats->push_back(stats_data.substr(0, open_parens_idx));
+  // Process name without parentheses.
+  proc_stats->push_back(
+      stats_data.substr(open_parens_idx + 1,
+                        close_parens_idx - (open_parens_idx + 1)));
+
+  // Split the rest.
+  std::vector<std::string> other_stats;
+  base::SplitString(stats_data.substr(close_parens_idx + 2), ' ', &other_stats);
+  for (size_t i = 0; i < other_stats.size(); ++i)
+    proc_stats->push_back(other_stats[i]);
+  return true;
+}
+
+// Reads the |field_num|th field from |proc_stats|. Returns 0 on failure.
+// This version does not handle the first 3 values, since the first value is
+// simply |pid|, and the next two values are strings.
+int GetProcStatsFieldAsInt(const std::vector<std::string>& proc_stats,
+                           ProcStatsFields field_num) {
+  if (field_num < VM_PPID) {
+    NOTREACHED();
+    return 0;
+  }
+
+  if (proc_stats.size() > static_cast<size_t>(field_num)) {
+    int value;
+    if (base::StringToInt(proc_stats[field_num], &value))
+      return value;
+  }
+  NOTREACHED();

[Jeffrey Yasskin, 2012/09/27 23:33:07]

This NOTREACHED gets hit when a field in proc is too big for an int. My
`unit_tests` run just crashed when proc_stats[VM_VSIZE]=="3073200128", which
isn't a crazy value for it to have.

+  return 0;
+}
+
+// Convenience wrapper around GetProcStatsFieldAsInt(), ParseProcStats() and
+// ReadProcStats(). See GetProcStatsFieldAsInt() for details.
+int ReadProcStatsAndGetFieldAsInt(pid_t pid, ProcStatsFields field_num) {
+  std::string stats_data;
+  if (!ReadProcStats(pid, &stats_data))
+    return 0;
+  std::vector<std::string> proc_stats;
+  if (!ParseProcStats(stats_data, &proc_stats))
+    return 0;
+  return GetProcStatsFieldAsInt(proc_stats, field_num);
+}
+
+// Reads the |field_num|th field from |proc_stats|.
+// Returns an empty string on failure.
+// This version only handles VM_COMM and VM_STATE, which are the only fields
+// that are strings.
+std::string GetProcStatsFieldAsString(
+    const std::vector<std::string>& proc_stats,
+    ProcStatsFields field_num) {
+  if (field_num < VM_COMM || field_num > VM_STATE) {
+    NOTREACHED();
+    return "";
+  }
+
+  if (proc_stats.size() > static_cast<size_t>(field_num))
+    return proc_stats[field_num];
+
+  NOTREACHED();
+  return 0;
+}
+
 // Reads /proc/<pid>/cmdline and populates |proc_cmd_line_args| with the command
 // line arguments. Returns true if successful.
 // Note: /proc/<pid>/cmdline contains command line arguments separated by single
@@ -71,28 +165,53 @@
   return true;
 }
 
+// Take a /proc directory entry named |d_name|, and if it is the directory for
+// a process, convert it to a pid_t.
+// Returns 0 on failure.
+// e.g. /proc/self/ will return 0, whereas /proc/1234 will return 1234.
+pid_t ProcDirSlotToPid(const char* d_name) {
+  int i;
+  for (i = 0; i < NAME_MAX && d_name[i]; ++i) {
+    if (!IsAsciiDigit(d_name[i])) {
+      return 0;
+    }
+  }
+  if (i == NAME_MAX)
+    return 0;
+
+  // Read the process's command line.
+  pid_t pid;
+  std::string pid_string(d_name);
+  if (!base::StringToInt(pid_string, &pid)) {
+    NOTREACHED();
+    return 0;
+  }
+  return pid;
+}
+
 // Get the total CPU of a single process.  Return value is number of jiffies
 // on success or -1 on error.
 int GetProcessCPU(pid_t pid) {
-  // Synchronously reading files in /proc is safe.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
-
   // Use /proc/<pid>/task to find all threads and parse their /stat file.
-  FilePath path = GetProcPidDir(pid).Append("task");
+  FilePath task_path = GetProcPidDir(pid).Append("task");
 
-  DIR* dir = opendir(path.value().c_str());
+  DIR* dir = opendir(task_path.value().c_str());
   if (!dir) {
-    DPLOG(ERROR) << "opendir(" << path.value() << ")";
+    DPLOG(ERROR) << "opendir(" << task_path.value() << ")";
     return -1;
   }
 
   int total_cpu = 0;
   while (struct dirent* ent = readdir(dir)) {
-    if (ent->d_name[0] == '.')
+    pid_t tid = ProcDirSlotToPid(ent->d_name);
+    if (!tid)
       continue;
 
-    FilePath stat_path = path.AppendASCII(ent->d_name).AppendASCII("stat");
+    // Synchronously reading files in /proc is safe.
+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+
     std::string stat;
+    FilePath stat_path = task_path.Append(ent->d_name).Append(kStatFile);
     if (file_util::ReadFileToString(stat_path, &stat)) {
       int cpu = base::ParseProcStatCPU(stat);
       if (cpu > 0)
@@ -104,22 +223,21 @@
   return total_cpu;
 }
 
-}  // namespace
-
-namespace base {
-
-ProcessId GetParentProcessId(ProcessHandle process) {
-  // Synchronously reading files in /proc is safe.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
-
-  FilePath stat_file = GetProcPidDir(process).Append("status");
+// Read /proc/<pid>/status and returns the value for |field|, or 0 on failure.
+// Only works for fields in the form of "Field: value kB".
+int ReadProcStatusAndGetFieldAsInt(pid_t pid, const std::string& field) {
+  FilePath stat_file = GetProcPidDir(pid).Append("status");
   std::string status;
-  if (!file_util::ReadFileToString(stat_file, &status))
-    return -1;
+  {
+    // Synchronously reading files in /proc is safe.
+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+    if (!file_util::ReadFileToString(stat_file, &status))
+      return 0;
+  }
 
   StringTokenizer tokenizer(status, ":\n");
   ParsingState state = KEY_NAME;
-  StringPiece last_key_name;
+  base::StringPiece last_key_name;
   while (tokenizer.GetNext()) {
     switch (state) {
       case KEY_NAME:
@@ -128,16 +246,40 @@
         break;
       case KEY_VALUE:
         DCHECK(!last_key_name.empty());
-        if (last_key_name == "PPid") {
-          int ppid;
-          base::StringToInt(tokenizer.token_piece(), &ppid);
-          return ppid;
+        if (last_key_name == field) {
+          std::string value_str;
+          tokenizer.token_piece().CopyToString(&value_str);
+          std::string value_str_trimmed;
+          TrimWhitespaceASCII(value_str, TRIM_ALL, &value_str_trimmed);
+          std::vector<std::string> split_value_str;
+          base::SplitString(value_str_trimmed, ' ', &split_value_str);
+          if (split_value_str.size() != 2 || split_value_str[1] != "kB") {
+            NOTREACHED();
+            return 0;
+          }
+          int value;
+          if (!base::StringToInt(split_value_str[0], &value)) {
+            NOTREACHED();
+            return 0;
+          }
+          return value;
         }
         state = KEY_NAME;
         break;
     }
   }
   NOTREACHED();
+  return 0;
+}
+
+}  // namespace
+
+namespace base {
+
+ProcessId GetParentProcessId(ProcessHandle process) {
+  ProcessId pid = ReadProcStatsAndGetFieldAsInt(process, VM_PPID);
+  if (pid)
+    return pid;
   return -1;
 }
 
@@ -166,66 +308,45 @@
 bool ProcessIterator::CheckForNextProcess() {
   // TODO(port): skip processes owned by different UID
 
-  dirent* slot = 0;
-  const char* openparen;
-  const char* closeparen;
+  pid_t pid = kNullProcessId;
   std::vector<std::string> cmd_line_args;
+  std::string stats_data;
+  std::vector<std::string> proc_stats;
 
   // Arbitrarily guess that there will never be more than 200 non-process
-  // files in /proc.  Hardy has 53.
+  // files in /proc.  Hardy has 53 and Lucid has 61.
   int skipped = 0;
   const int kSkipLimit = 200;
   while (skipped < kSkipLimit) {
-    slot = readdir(procfs_dir_);
+    dirent* slot = readdir(procfs_dir_);
     // all done looking through /proc?
     if (!slot)
       return false;
 
     // If not a process, keep looking for one.
-    bool notprocess = false;
-    int i;
-    for (i = 0; i < NAME_MAX && slot->d_name[i]; ++i) {
-       if (!isdigit(slot->d_name[i])) {
-         notprocess = true;
-         break;
-       }
-    }
-    if (i == NAME_MAX || notprocess) {
+    pid = ProcDirSlotToPid(slot->d_name);
+    if (!pid) {
       skipped++;
       continue;
     }
 
-    // Read the process's command line.
-    std::string pid_string(slot->d_name);
-    int pid;
-    if (StringToInt(pid_string, &pid) && !GetProcCmdline(pid, &cmd_line_args))
+    if (!GetProcCmdline(pid, &cmd_line_args))
       continue;
 
-    // Read the process's status.
-    char buf[NAME_MAX + 12];
-    sprintf(buf, "/proc/%s/stat", slot->d_name);
-    FILE* fp = fopen(buf, "r");
-    if (!fp)
+    if (!ReadProcStats(pid, &stats_data))
       continue;
-    const char* result = fgets(buf, sizeof(buf), fp);
-    fclose(fp);
-    if (!result)
+    if (!ParseProcStats(stats_data, &proc_stats))
       continue;
 
-    // Parse the status.  It is formatted like this:
-    // %d (%s) %c %d %d ...
-    // pid (name) runstate ppid gid
-    // To avoid being fooled by names containing a closing paren, scan
-    // backwards.
-    openparen = strchr(buf, '(');
-    closeparen = strrchr(buf, ')');
-    if (!openparen || !closeparen)
+    std::string runstate = GetProcStatsFieldAsString(proc_stats, VM_STATE);
+    if (runstate.size() != 1) {
+      NOTREACHED();
       continue;
-    char runstate = closeparen[2];
+    }
 
     // Is the process in 'Zombie' state, i.e. dead but waiting to be reaped?
     // Allowed values: D R S T Z
-    if (runstate != 'Z')
+    if (runstate[0] != 'Z')
       break;
 
     // Nope, it's a zombie; somebody isn't cleaning up after their children.
@@ -237,17 +358,14 @@
     return false;
   }
 
-  // This seems fragile.
-  entry_.pid_ = atoi(slot->d_name);
-  entry_.ppid_ = atoi(closeparen + 3);
-  entry_.gid_ = atoi(strchr(closeparen + 4, ' '));
-
+  entry_.pid_ = pid;
+  entry_.ppid_ = GetProcStatsFieldAsInt(proc_stats, VM_PPID);
+  entry_.gid_ = GetProcStatsFieldAsInt(proc_stats, VM_PGRP);
   entry_.cmd_line_args_.assign(cmd_line_args.begin(), cmd_line_args.end());
 
   // TODO(port): read pid's commandline's $0, like killall does.  Using the
   // short name between openparen and closeparen won't work for long names!
-  int len = closeparen - openparen - 1;
-  entry_.exe_file_.assign(openparen + 1, len);
+  entry_.exe_file_ = GetProcStatsFieldAsString(proc_stats, VM_COMM);
   return true;
 }
 
@@ -265,58 +383,22 @@
 
 // On linux, we return vsize.
 size_t ProcessMetrics::GetPagefileUsage() const {
-  std::vector<std::string> proc_stats;
-  if (!GetProcStats(process_, &proc_stats))
-    DLOG(WARNING) << "Failed to get process stats.";
-  const size_t kVmSize = 22;
-  if (proc_stats.size() > kVmSize) {
-    int vm_size;
-    base::StringToInt(proc_stats[kVmSize], &vm_size);
-    return static_cast<size_t>(vm_size);
-  }
-  return 0;
+  return ReadProcStatsAndGetFieldAsInt(process_, VM_VSIZE);
 }
 
 // On linux, we return the high water mark of vsize.
 size_t ProcessMetrics::GetPeakPagefileUsage() const {
-  std::vector<std::string> proc_stats;
-  if (!GetProcStats(process_, &proc_stats))
-    DLOG(WARNING) << "Failed to get process stats.";
-  const size_t kVmPeak = 21;
-  if (proc_stats.size() > kVmPeak) {
-    int vm_peak;
-    if (base::StringToInt(proc_stats[kVmPeak], &vm_peak))
-      return vm_peak;
-  }
-  return 0;
+  return ReadProcStatusAndGetFieldAsInt(process_, "VmPeak") * 1024;
 }
 
 // On linux, we return RSS.
 size_t ProcessMetrics::GetWorkingSetSize() const {
-  std::vector<std::string> proc_stats;
-  if (!GetProcStats(process_, &proc_stats))
-    DLOG(WARNING) << "Failed to get process stats.";
-  const size_t kVmRss = 23;
-  if (proc_stats.size() > kVmRss) {
-    int num_pages;
-    if (base::StringToInt(proc_stats[kVmRss], &num_pages))
-      return static_cast<size_t>(num_pages) * getpagesize();
-  }
-  return 0;
+  return ReadProcStatsAndGetFieldAsInt(process_, VM_RSS) * getpagesize();
 }
 
 // On linux, we return the high water mark of RSS.
 size_t ProcessMetrics::GetPeakWorkingSetSize() const {
-  std::vector<std::string> proc_stats;
-  if (!GetProcStats(process_, &proc_stats))
-    DLOG(WARNING) << "Failed to get process stats.";
-  const size_t kVmHwm = 23;
-  if (proc_stats.size() > kVmHwm) {
-    int num_pages;
-    base::StringToInt(proc_stats[kVmHwm], &num_pages);
-    return static_cast<size_t>(num_pages) * getpagesize();
-  }
-  return 0;
+  return ReadProcStatusAndGetFieldAsInt(process_, "VmHWM") * 1024;
 }
 
 bool ProcessMetrics::GetMemoryBytes(size_t* private_bytes,
@@ -326,7 +408,7 @@
     return false;
 
   if (private_bytes)
-    *private_bytes = ws_usage.priv << 10;
+    *private_bytes = ws_usage.priv * 1024;
 
   if (shared_bytes)
     *shared_bytes = ws_usage.shared * 1024;
@@ -342,7 +424,7 @@
 
   // First we need to get the page size, since everything is measured in pages.
   // For details, see: man 5 proc.
-  const int page_size_kb = sysconf(_SC_PAGE_SIZE) / 1024;
+  const int page_size_kb = getpagesize() / 1024;
   if (page_size_kb <= 0)
     return false;
 
@@ -473,24 +555,15 @@
 
 // Exposed for testing.
 int ParseProcStatCPU(const std::string& input) {
-  // /proc/<pid>/stat contains the process name in parens.  In case the
-  // process name itself contains parens, skip past them.
-  std::string::size_type rparen = input.rfind(')');
-  if (rparen == std::string::npos)
+  std::vector<std::string> proc_stats;
+  if (!ParseProcStats(input, &proc_stats))
     return -1;
 
-  // From here, we expect a bunch of space-separated fields, where the
-  // 0-indexed 11th and 12th are utime and stime.  On two different machines
-  // I found 42 and 39 fields, so let's just expect the ones we need.
-  std::vector<std::string> fields;
-  base::SplitString(input.substr(rparen + 2), ' ', &fields);
-  if (fields.size() < 13)
-    return -1;  // Output not in the format we expect.
-
-  int fields11, fields12;
-  base::StringToInt(fields[11], &fields11);
-  base::StringToInt(fields[12], &fields12);
-  return fields11 + fields12;
+  if (proc_stats.size() <= VM_STIME)
+    return -1;
+  int utime = GetProcStatsFieldAsInt(proc_stats, VM_UTIME);
+  int stime = GetProcStatsFieldAsInt(proc_stats, VM_STIME);
+  return utime + stime;
 }
 
 namespace {
@@ -519,15 +592,15 @@
   FilePath meminfo_file("/proc/meminfo");
   std::string meminfo_data;
   if (!file_util::ReadFileToString(meminfo_file, &meminfo_data)) {
-    DLOG(WARNING) << "Failed to open /proc/meminfo.";
+    DLOG(WARNING) << "Failed to open " << meminfo_file.value();
     return false;
   }
   std::vector<std::string> meminfo_fields;
   SplitStringAlongWhitespace(meminfo_data, &meminfo_fields);
 
   if (meminfo_fields.size() < kMemCachedIndex) {
-    DLOG(WARNING) << "Failed to parse /proc/meminfo.  Only found " <<
-      meminfo_fields.size() << " fields.";
+    DLOG(WARNING) << "Failed to parse " << meminfo_file.value()
+                  << ".  Only found " << meminfo_fields.size() << " fields.";
     return false;
   }