SERVER-20737 Don't pass nullptr to memcmp in StringData::compare

1 files changed, 36 insertions, 19 deletions

diff --git a/src/mongo/base/string_data.h b/src/mongo/base/string_data.h
index e3c58a747d4..e30bd6ca3fd 100644
--- a/src/mongo/base/string_data.h
+++ b/src/mongo/base/string_data.h
@@ -35,6 +35,10 @@
 #include <limits>
 #include <string>
 
+#define MONGO_INCLUDE_INVARIANT_H_WHITELISTED
+#include "mongo/util/invariant.h"
+#undef MONGO_INCLUDE_INVARIANT_H_WHITELISTED
+
 namespace mongo {
 
 /**
@@ -51,34 +55,47 @@ namespace mongo {
  *    rawData() terminates with a null.
  */
 class StringData {
+    struct TrustedInitTag {};
+    StringData(const char* c, size_t len, TrustedInitTag) : _data(c), _size(len) {}
+
 public:
-    /** Constructs an empty std::string data */
-    StringData() : _data(NULL), _size(0) {}
+    /** Constructs an empty StringData */
+    StringData() = default;
 
     /**
-     * Constructs a StringData, for the case where the length of std::string is not known. 'c'
-     * must be a pointer to a null-terminated string.
+     * Constructs a StringData, for the case where the length of the
+     * string is not known. 'c' must be a pointer to a null-terminated
+     * string.
      */
-    StringData(const char* str) : _data(str), _size((str == NULL) ? 0 : std::strlen(str)) {}
+    StringData(const char* str) : StringData(str, str ? std::strlen(str) : 0) {}
 
     /**
-     * Constructs a StringData explicitly, for the case where the length of the std::string is
-     * already known. 'c' must be a pointer to a null-terminated string, and len must
-     * be the length that strlen(c) would return, a.k.a the index of the terminator in c.
+     * Constructs a StringData explicitly, for the case of a literal
+     * whose size is known at compile time. Note that you probably
+     * don't need this on a modern compiler that can see that the call
+     * to std::strlen on StringData("foo") can be constexpr'ed out.
      */
-    StringData(const char* c, size_t len) : _data(c), _size(len) {}
+    struct LiteralTag {};
+    template <size_t N>
+    StringData(const char(&val)[N], LiteralTag)
+        : StringData(&val[0], N - 1) {}
 
-    /** Constructs a StringData, for the case of a string. */
-    StringData(const std::string& s) : _data(s.c_str()), _size(s.size()) {}
+    /** Constructs a StringData, for the case of a std::string. We can
+     * use the trusted init path with no follow on checks because
+     * string::data is assured to never return nullptr.
+     */
+    StringData(const std::string& s) : StringData(s.data(), s.length(), TrustedInitTag()) {}
 
     /**
-     * Constructs a StringData explicitly, for the case of a literal whose size is known at
-     * compile time.
+     * Constructs a StringData with an explicit length. 'c' must be a
+     * pointer into a character array. The StringData will refer to
+     * the first 'len' characters starting at 'c'. The range of
+     * characters c to c+len must be valid.
      */
-    struct LiteralTag {};
-    template <size_t N>
-    StringData(const char(&val)[N], LiteralTag)
-        : _data(&val[0]), _size(N - 1) {}
+    StringData(const char* c, size_t len) : StringData(c, len, TrustedInitTag()) {
+        if (kDebugBuild)
+            invariant(_data || (_size == 0));
+    }
 
     /**
      * Returns -1, 0, or 1 if 'this' is less, equal, or greater than 'other' in
@@ -164,8 +181,8 @@ public:
     }
 
 private:
-    const char* _data;  // is not guaranted to be null terminated (see "notes" above)
-    size_t _size;       // 'size' does not include the null terminator
+    const char* _data = nullptr;  // is not guaranted to be null terminated (see "notes" above)
+    size_t _size = 0;             // 'size' does not include the null terminator
 };
 
 inline bool operator==(StringData lhs, StringData rhs) {