logging: treat `/` and `\\` as category name separators

Summary:
Update the LogName code to also treat `/` and `\\` as category separators, just
like `.`.

This makes it easy to use directory and file names directly as log category
names.  The XLOG() code already turns file names in to log category names.
Previously `getXlogCategoryNameForFile()` had to do its own translation of
directory separators into `.`.  Users manually had to perform this translation
on their own when setting the configuration for specific categories.

This change makes it possible for users to specify the filename as input
directly when updating the logging configuration and it automatically updates
the correct category.

This also allows us to avoid allocating a new string in
`getXlogCategoryNameForFile()` since we no longer have to translate directory
separator characters there.

Reviewed By: chadaustin

Differential Revision: D7348520

fbshipit-source-id: 0e996f4351798b4636786a14ebb2d8820a2cc153

folly/experimental/logging/LogName.cpp

@@ -15,85 +15,109 @@
  */
 #include <folly/experimental/logging/LogName.h>
 
+namespace {
+constexpr bool isSeparator(char c) {
+  return c == '.' || c == '/' || c == '\\';
+}
+} // namespace
+
 namespace folly {
 
 std::string LogName::canonicalize(StringPiece input) {
   std::string cname;
   cname.reserve(input.size());
 
-  // Ignore trailing '.'s
+  // Ignore trailing category separator characters
   size_t end = input.size();
-  while (end > 0 && input[end - 1] == '.') {
+  while (end > 0 && isSeparator(input[end - 1])) {
     --end;
   }
 
-  bool ignoreDot = true;
+  bool ignoreSeparator = true;
   for (size_t idx = 0; idx < end; ++idx) {
-    if (ignoreDot && input[idx] == '.') {
+    if (isSeparator(input[idx])) {
+      if (ignoreSeparator) {
         continue;
       }
+      cname.push_back('.');
+      ignoreSeparator = true;
+    } else {
       cname.push_back(input[idx]);
-    ignoreDot = (input[idx] == '.');
+      ignoreSeparator = false;
+    }
   }
   return cname;
 }
 
 size_t LogName::hash(StringPiece name) {
-  // Code based on StringPiece::hash(), but which ignores leading and
-  // trailing '.' characters, as well as multiple consecutive '.' characters,
-  // so equivalent names result in the same hash.
+  // Code based on StringPiece::hash(), but which ignores leading and trailing
+  // category separator characters, as well as multiple consecutive separator
+  // characters, so equivalent names result in the same hash.
   uint32_t hash = 5381;
 
   size_t end = name.size();
-  while (end > 0 && name[end - 1] == '.') {
+  while (end > 0 && isSeparator(name[end - 1])) {
     --end;
   }
 
-  bool ignoreDot = true;
+  bool ignoreSeparator = true;
   for (size_t idx = 0; idx < end; ++idx) {
-    if (ignoreDot && name[idx] == '.') {
+    uint8_t value;
+    if (isSeparator(name[idx])) {
+      if (ignoreSeparator) {
         continue;
       }
-    hash = ((hash << 5) + hash) + name[idx];
-    // If this character was a '.', ignore subsequent consecutive '.'s
-    ignoreDot = (name[idx] == '.');
+      value = '.';
+      ignoreSeparator = true;
+    } else {
+      value = static_cast<uint8_t>(name[idx]);
+      ignoreSeparator = false;
+    }
+    hash = ((hash << 5) + hash) + value;
   }
   return hash;
 }
 
 int LogName::cmp(StringPiece a, StringPiece b) {
-  // Ignore trailing '.'s
-  auto stripTrailingDots = [](StringPiece& s) {
-    while (!s.empty() && s.back() == '.') {
+  // Ignore trailing separators
+  auto stripTrailingSeparators = [](StringPiece& s) {
+    while (!s.empty() && isSeparator(s.back())) {
       s.uncheckedSubtract(1);
     }
   };
-  stripTrailingDots(a);
-  stripTrailingDots(b);
+  stripTrailingSeparators(a);
+  stripTrailingSeparators(b);
 
-  // Advance ptr until it no longer points to a '.'
-  // This is used to skip over consecutive sequences of '.' characters.
-  auto skipOverDots = [](StringPiece& s) {
-    while (!s.empty() && s.front() == '.') {
+  // Advance ptr until it no longer points to a category separator.
+  // This is used to skip over consecutive sequences of separator characters.
+  auto skipOverSeparators = [](StringPiece& s) {
+    while (!s.empty() && isSeparator(s.front())) {
       s.uncheckedAdvance(1);
     }
   };
 
-  bool ignoreDot = true;
+  bool ignoreSeparator = true;
   while (true) {
-    if (ignoreDot) {
-      skipOverDots(a);
-      skipOverDots(b);
+    if (ignoreSeparator) {
+      skipOverSeparators(a);
+      skipOverSeparators(b);
     }
     if (a.empty()) {
       return b.empty() ? 0 : -1;
     } else if (b.empty()) {
       return 1;
     }
+    if (isSeparator(a.front())) {
+      if (!isSeparator(b.front())) {
+        return '.' - b.front();
+      }
+      ignoreSeparator = true;
+    } else {
       if (a.front() != b.front()) {
         return a.front() - b.front();
       }
-    ignoreDot = (a.front() == '.');
+      ignoreSeparator = false;
+    }
     a.uncheckedAdvance(1);
     b.uncheckedAdvance(1);
   }
@@ -106,19 +130,19 @@ StringPiece LogName::getParent(StringPiece name) {
 
   ssize_t idx = name.size();
 
-  // Skip over any trailing '.' characters
-  while (idx > 0 && name[idx - 1] == '.') {
+  // Skip over any trailing separator characters
+  while (idx > 0 && isSeparator(name[idx - 1])) {
     --idx;
   }
 
-  // Now walk backwards to the next '.' character
-  while (idx > 0 && name[idx - 1] != '.') {
+  // Now walk backwards to the next separator character
+  while (idx > 0 && !isSeparator(name[idx - 1])) {
     --idx;
   }
 
-  // And again skip over any '.' characters, in case there are multiple
+  // And again skip over any separator characters, in case there are multiple
   // repeated characters.
-  while (idx > 0 && name[idx - 1] == '.') {
+  while (idx > 0 && isSeparator(name[idx - 1])) {
     --idx;
   }
 

folly/experimental/logging/LogName.h

@@ -23,16 +23,17 @@ namespace folly {
  * The LogName class contains utility functions for processing log category
  * names.  It primarily handles canonicalization of names.
  *
- * For instance, "foo.bar", "foo..bar", and ".foo.bar..." all refer to the same
- * log category.
+ * For instance, "foo.bar", "foo/bar", "foo..bar", and ".foo.bar..." all refer
+ * to the same log category.
  */
 class LogName {
  public:
   /**
    * Return a canonicalized version of the log name.
    *
-   * Leading and trailing '.' characters are removed, and all sequences of
-   * consecutive '.' characters are replaced with a single '.'
+   * '/' and '\\' characters are converted to '.', then leading and trailing
+   * '.' characters are removed, and all sequences of consecutive '.'
+   * characters are replaced with a single '.'
    */
   static std::string canonicalize(folly::StringPiece name);
 

folly/experimental/logging/test/LogNameTest.cpp

@@ -21,17 +21,29 @@ using namespace folly;
 TEST(LogName, canonicalize) {
   EXPECT_EQ("", LogName::canonicalize("."));
   EXPECT_EQ("", LogName::canonicalize("..."));
+  EXPECT_EQ("", LogName::canonicalize("/"));
+  EXPECT_EQ("", LogName::canonicalize("\\"));
+  EXPECT_EQ("", LogName::canonicalize(".//..\\\\./"));
   EXPECT_EQ("foo.bar", LogName::canonicalize(".foo..bar."));
   EXPECT_EQ("a.b.c", LogName::canonicalize("a.b.c"));
+  EXPECT_EQ("a.b.c", LogName::canonicalize("a/b/c"));
+  EXPECT_EQ("a.b.c", LogName::canonicalize("a/b/c/"));
   EXPECT_EQ("a.b.c", LogName::canonicalize("a..b.c..."));
   EXPECT_EQ("a.b.c", LogName::canonicalize("....a.b.c"));
   EXPECT_EQ("a.b.c", LogName::canonicalize("a.b.c...."));
+  EXPECT_EQ("a.b.c", LogName::canonicalize("////a.b.c"));
+  EXPECT_EQ("a.b.c", LogName::canonicalize("a.b.c////"));
+  EXPECT_EQ("a.b.c", LogName::canonicalize("/a.b.//.c/"));
 }
 
 TEST(LogName, getParent) {
   EXPECT_EQ("", LogName::getParent("foo"));
   EXPECT_EQ("", LogName::getParent(".foo"));
   EXPECT_EQ("foo", LogName::getParent("foo.bar"));
+  EXPECT_EQ("foo", LogName::getParent("foo/bar"));
+  EXPECT_EQ("foo", LogName::getParent("foo\\bar"));
+  EXPECT_EQ("foo", LogName::getParent("foo\\bar/"));
+  EXPECT_EQ("foo", LogName::getParent("foo\\bar\\"));
   EXPECT_EQ("foo..bar", LogName::getParent("foo..bar..test"));
   EXPECT_EQ("..foo..bar", LogName::getParent("..foo..bar..test.."));
 }
@@ -40,7 +52,11 @@ TEST(LogName, hash) {
   EXPECT_EQ(LogName::hash("foo"), LogName::hash("foo."));
   EXPECT_EQ(LogName::hash(".foo..bar"), LogName::hash("foo.bar..."));
   EXPECT_EQ(LogName::hash("a.b.c..d."), LogName::hash("..a.b.c.d."));
+  EXPECT_EQ(LogName::hash("a.b.c.d"), LogName::hash("/a/b/c/d/"));
+  EXPECT_EQ(LogName::hash("a.b.c.d"), LogName::hash("a\\b\\c/d/"));
   EXPECT_EQ(LogName::hash(""), LogName::hash("."));
+  EXPECT_EQ(LogName::hash(""), LogName::hash("//"));
+  EXPECT_EQ(LogName::hash(""), LogName::hash("\\"));
   EXPECT_EQ(LogName::hash(""), LogName::hash("...."));
 
   // Hashes for different category names should generally be different.
@@ -52,9 +68,11 @@ TEST(LogName, hash) {
 
 TEST(LogName, cmp) {
   EXPECT_EQ(0, LogName::cmp("foo", "foo."));
+  EXPECT_EQ(0, LogName::cmp("foo", "foo/"));
   EXPECT_EQ(0, LogName::cmp(".foo..bar", "foo.bar..."));
   EXPECT_EQ(0, LogName::cmp(".foo.bar", "foo...bar..."));
   EXPECT_EQ(0, LogName::cmp("a.b.c..d.", "..a.b.c.d."));
+  EXPECT_EQ(0, LogName::cmp("a.b.c..d.", "\\/a.b/c/d."));
   EXPECT_EQ(0, LogName::cmp("", "."));
   EXPECT_EQ(0, LogName::cmp("", "...."));
 

folly/experimental/logging/test/XlogTest.cpp

@@ -191,20 +191,27 @@ TEST(Xlog, getXlogCategoryName) {
   EXPECT_EQ("foo.cpp", getXlogCategoryNameForFile("foo.cpp"));
   EXPECT_EQ("foo.h", getXlogCategoryNameForFile("foo.h"));
 
-  // Directory separators should be translated to "."
-  EXPECT_EQ("src.test.foo.cpp", getXlogCategoryNameForFile("src/test/foo.cpp"));
-  EXPECT_EQ("src.test.foo.h", getXlogCategoryNameForFile("src/test/foo.h"));
+  // Directory separators should be translated to "." during LogName
+  // canonicalization
+  EXPECT_EQ("src/test/foo.cpp", getXlogCategoryNameForFile("src/test/foo.cpp"));
+  EXPECT_EQ(
+      "src.test.foo.cpp",
+      LogName::canonicalize(getXlogCategoryNameForFile("src/test/foo.cpp")));
+  EXPECT_EQ("src/test/foo.h", getXlogCategoryNameForFile("src/test/foo.h"));
+  EXPECT_EQ(
+      "src.test.foo.h",
+      LogName::canonicalize(getXlogCategoryNameForFile("src/test/foo.h")));
 
   // Buck's directory prefixes for generated source files
   // should be stripped out
   EXPECT_EQ(
       "myproject.generated_header.h",
-      getXlogCategoryNameForFile(
-          "buck-out/gen/myproject#headers/myproject/generated_header.h"));
+      LogName::canonicalize(getXlogCategoryNameForFile(
+          "buck-out/gen/myproject#headers/myproject/generated_header.h")));
   EXPECT_EQ(
       "foo.bar.test.h",
-      getXlogCategoryNameForFile(
-          "buck-out/gen/foo/bar#header-map,headers/foo/bar/test.h"));
+      LogName::canonicalize(getXlogCategoryNameForFile(
+          "buck-out/gen/foo/bar#header-map,headers/foo/bar/test.h")));
 }
 
 TEST(Xlog, xlogStripFilename) {

folly/experimental/logging/xlog.cpp

@@ -54,7 +54,7 @@ StringPiece stripBuckOutPrefix(StringPiece filename) {
 }
 } // namespace
 
-std::string getXlogCategoryNameForFile(StringPiece filename) {
+StringPiece getXlogCategoryNameForFile(StringPiece filename) {
   // Buck mangles the directory layout for header files.  Rather than including
   // them from their original location, it moves them into deep directories
   // inside buck-out, and includes them from there.
@@ -65,17 +65,7 @@ std::string getXlogCategoryNameForFile(StringPiece filename) {
     filename = stripBuckOutPrefix(filename);
   }
 
-  std::string categoryName = filename.str();
-
-  // Translate slashes to dots, to turn the directory layout into
-  // a category hierarchy.
-  for (size_t n = 0; n < categoryName.size(); ++n) {
-    if (xlogIsDirSeparator(categoryName[n])) {
-      categoryName[n] = '.';
-    }
-  }
-
-  return categoryName;
+  return filename;
 }
 
 template <bool IsInHeaderFile>

folly/experimental/logging/xlog.h

@@ -399,7 +399,7 @@ class XlogCategoryInfo<false> {
  * This function returns the category name that will be used by XLOG() if
  * XLOG_SET_CATEGORY_NAME() has not been used.
  */
-std::string getXlogCategoryNameForFile(folly::StringPiece filename);
+folly::StringPiece getXlogCategoryNameForFile(folly::StringPiece filename);
 
 constexpr bool xlogIsDirSeparator(char c) {
   return c == '/' || (kIsWindows && c == '\\');