Parsing CPU info on Linux reads only cpuinfo file

[mvafin]

There is a bug in a file sysinfo.cc in a function ReadIntFromFile which is used to read CPU frequency from files /sys/devices/system/cpu/cpu0/tsc_freq_khz and /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq. Here is the code of the function:

// Helper function for reading an int from a file. Returns true if successful
// and the memory location pointed to by value is set to the value read.
bool ReadIntFromFile(const char* file, long* value) {
  bool ret = false;
  int fd = open(file, O_RDONLY);
  if (fd != -1) {
    char line[1024];
    char* err;
    memset(line, '\0', sizeof(line));
    CHECK(read(fd, line, sizeof(line) - 1));
    const long temp_value = strtol(line, &err, 10);
    if (line[0] != '\0' && (*err == '\n' || *err == '\0')) {
      *value = temp_value;
      ret = true;
    }
    close(fd);
  }
  return ret;
}

Macro CHECK usage is wrong. Here is its code:

// The CHECK macro returns a std::ostream object that can have extra information
// written to it.
#ifndef NDEBUG
#define CHECK(b)                                                             \
  (b ? ::benchmark::internal::GetNullLogInstance()                           \
     : ::benchmark::internal::CheckHandler(#b, __FILE__, __func__, __LINE__) \
           .GetLog())
#else
#define CHECK(b) ::benchmark::internal::GetNullLogInstance()
#endif

This bug results in no call to read(fd, line, sizeof(line) - 1) function in release mode when NDEBUG is defined.
This was discovered when I tried to run the benchmark on Raspberry Pi 3 with Ubuntu installed on it and got 77 MHz frequency, while it has 1200 MHz. It was because BogoMIPS frequency was used from /proc/cpuinfo file while /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq had the right frequency in it. I will submit a patch for this issue shortly.

[EricWF]

Could you put the body of this issue into the description of the pull-request? That way it's present in the git commit logs.

Additionally I'm closing this issue since there is a PR tracking it. Thanks for the report and for the fix.