KernelVersion supports more version formats

Summary: This doesn't handle every possible case, but should handle most common cases. We can prevent more crashes like occurred in facebook#3513

Fixes facebook#3979

Reviewed By: @ptarjan

Differential Revision: D1617809

hphp/util/kernel-version.cpp

@@ -20,35 +20,46 @@
 #include <unistd.h>
 #include <sys/utsname.h>
 #include <stdio.h>
-
+#include <iostream>
 namespace HPHP {
 
 void KernelVersion::parse(const char* s) {
-  int numFields = sscanf(s,
-                         "%d.%d.%d-%d_fbk%d_",
-                         &m_major, &m_dot, &m_dotdot, &m_dash,
-                         &m_fbk);
-  // The first release in each series is x.y, not x.y.0, so we need a different
-  // pattern.
-  if (numFields == 2) {
-    m_dotdot = 0;
-    numFields = sscanf(s,
-                       "%d.%d-%d_fbk%d_",
-                       &m_major, &m_dot, &m_dash, &m_fbk);
+  char dashdot[1];
+  // Assume no part of the uname string will be more than 128 chars
+  char release[128];
+  release[0] = 0;
+  char build[128];
+  build[0] = 0;
+  sscanf(s,
+         "%d.%d%[.-]%[A-Za-z0-9]-%[A-Za-z0-9]_fbk%d_",
+         &m_major, &m_minor, dashdot, release, build, &m_fbk);
+  assert(m_major > 0 && m_minor > 0);
+  m_release_str = release[0] == 0 ? "" : (const char*) release;
+  m_build_str = build[0] == 0 ? "" : (const char*) build;
+  // Populate the int-based release and build, if we have all digits in them
+  if (isNumber(m_release_str)) {
+    m_release = atoi(m_release_str.c_str());
+  }
+  if (isNumber(m_build_str)) {
+    m_build = atoi(m_build_str.c_str());
   }
-  assert(numFields >= 3);
 }
 
 KernelVersion::KernelVersion() {
   struct utsname uts;
   DEBUG_ONLY int err = uname(&uts);
   assert(err == 0);
-  m_major = m_dot = m_dotdot = m_dash = m_fbk = -1;
   parse(uts.release);
 }
 
 KernelVersion::KernelVersion(const char* s) {
+  m_major = m_minor = m_fbk = m_release = m_build = -1;
+  m_release_str = m_build_str = "";
   parse(s);
 }
 
+bool KernelVersion::isNumber(const std::string s) {
+  return !s.empty() && s.find_first_not_of("0123456789") == std::string::npos;
+}
+
 }

hphp/util/kernel-version.h

@@ -16,29 +16,34 @@
 #ifndef incl_HPHP_KERNEL_VERSION_H_
 #define incl_HPHP_KERNEL_VERSION_H_
 
+#include <string>
+
 namespace HPHP {
 
 struct KernelVersion {
-  // <major>.<dot>.<dotdot>-<dash>_fbk<fbk>
+  // <major>.<minor>.<release>-<build>_fbk<fbk>
   int m_major;
-  int m_dot;
-  int m_dotdot;
-  int m_dash;
+  int m_minor;
+  int m_release;
+  int m_build;
+  std::string m_release_str;
+  std::string m_build_str;
   int m_fbk;
   KernelVersion();             // Use uname
   explicit KernelVersion(const char*);  // A known kernel version for cmp.
   static int cmp(const KernelVersion& l, const KernelVersion& r) {
 #define C(field) if (l.field != r.field) return l.field - r.field;
     C(m_major);
-    C(m_dot);
-    C(m_dotdot);
-    C(m_dash);
+    C(m_minor);
+    C(m_release);
+    C(m_build);
     C(m_fbk);
 #undef C
     return 0;
   }
   private:
   void parse(const char* c);
+  bool isNumber(const std::string s);
 };
 
 }

hphp/util/test/kernel-version.cpp

@@ -21,10 +21,52 @@ namespace HPHP {
 TEST(KernelVersion, SimpleTest) {
   KernelVersion minKv("3.2.28-72_fbk12");
   EXPECT_EQ(3, minKv.m_major);
-  EXPECT_EQ(2, minKv.m_dot);
-  EXPECT_EQ(28, minKv.m_dotdot);
-  EXPECT_EQ(72, minKv.m_dash);
+  EXPECT_EQ(2, minKv.m_minor);
+  EXPECT_EQ(28, minKv.m_release);
+  EXPECT_EQ(72, minKv.m_build);
+  EXPECT_EQ(0, strcmp(minKv.m_release_str.c_str(), "28"));
+  EXPECT_EQ(0, strcmp(minKv.m_build_str.c_str(), "72"));
   EXPECT_EQ(12, minKv.m_fbk);
+  KernelVersion minKvB("3.14-1-amd64_fbk64");
+  EXPECT_EQ(3, minKvB.m_major);
+  EXPECT_EQ(14, minKvB.m_minor);
+  EXPECT_EQ(1, minKvB.m_release);
+  EXPECT_EQ(-1, minKvB.m_build);
+  EXPECT_EQ(0, strcmp(minKvB.m_release_str.c_str(), "1"));
+  EXPECT_EQ(0, strcmp(minKvB.m_build_str.c_str(), "amd64"));
+  EXPECT_EQ(64, minKvB.m_fbk);
+  KernelVersion minKvC("3.16-trunk-amd64");
+  EXPECT_EQ(3, minKvC.m_major);
+  EXPECT_EQ(16, minKvC.m_minor);
+  EXPECT_EQ(-1, minKvC.m_release);
+  EXPECT_EQ(-1, minKvC.m_build);
+  EXPECT_EQ(0, strcmp(minKvC.m_release_str.c_str(), "trunk"));
+  EXPECT_EQ(0, strcmp(minKvC.m_build_str.c_str(), "amd64"));
+  EXPECT_EQ(-1, minKvC.m_fbk);
+  KernelVersion minKvD("3.16-trunk-amd64_fbk12");
+  EXPECT_EQ(3, minKvD.m_major);
+  EXPECT_EQ(16, minKvD.m_minor);
+  EXPECT_EQ(-1, minKvD.m_release);
+  EXPECT_EQ(-1, minKvD.m_build);
+  EXPECT_EQ(0, strcmp(minKvD.m_release_str.c_str(), "trunk"));
+  EXPECT_EQ(0, strcmp(minKvD.m_build_str.c_str(), "amd64"));
+  EXPECT_EQ(12, minKvD.m_fbk);
+  KernelVersion minKvE("3.14-1-i386");
+  EXPECT_EQ(3, minKvE.m_major);
+  EXPECT_EQ(14, minKvE.m_minor);
+  EXPECT_EQ(1, minKvE.m_release);
+  EXPECT_EQ(-1, minKvE.m_build);
+  EXPECT_EQ(0, strcmp(minKvE.m_release_str.c_str(), "1"));
+  EXPECT_EQ(0, strcmp(minKvE.m_build_str.c_str(), "i386"));
+  EXPECT_EQ(-1, minKvE.m_fbk);
+  KernelVersion minKvF("3.14");
+  EXPECT_EQ(3, minKvF.m_major);
+  EXPECT_EQ(14, minKvF.m_minor);
+  EXPECT_EQ(-1, minKvF.m_release);
+  EXPECT_EQ(-1, minKvF.m_build);
+  EXPECT_EQ(0, strcmp(minKvF.m_release_str.c_str(), ""));
+  EXPECT_EQ(0, strcmp(minKvF.m_build_str.c_str(), ""));
+  EXPECT_EQ(-1, minKvF.m_fbk);
 }
 
 }