Refactor der::Input helper methods into new constructors

net/der/input.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_DER_INPUT_H_
 #define NET_DER_INPUT_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <string>
 
 #include "base/compiler_specific.h"
+#include "base/strings/string_piece.h"
 #include "net/base/net_export.h"
 
 namespace net {
 
 namespace der {
 
 class Mark;
 
 // An opaque class that represents a fixed buffer of data of a fixed length,
 // to be used as an input to other operations. An Input object does not own
@@ skipping 20 matching lines @@
   Input();
 
   // Creates an Input from a constant array |data|.
   template <size_t N>
   explicit Input(const uint8_t(&data)[N])
       : data_(data), len_(N) {}
 
   // Creates an Input from the given |data| and |len|.
   Input(const uint8_t* data, size_t len);
 
+  // Creates an Input from a base::StringPiece.
+  Input(const base::StringPiece& sp);

[eroman, 2016/01/13 22:54:00]

Mark as explicit

[nharper, 2016/01/13 23:04:47]

Done.

+
+  // Creates an Input from a std::string. The lifetimes are a bit subtle when
+  // using this function: The constructed Input is only valid so long as |s| is
+  // still alive and not mutated.
+  Input(const std::string* s);

[eroman, 2016/01/13 22:54:00]

explicit

[nharper, 2016/01/13 23:04:48]

Done.

+
   // Returns the length in bytes of an Input's data.
   size_t Length() const { return len_; }
 
   // Return true if the Input's data and |other|'s data are byte-wise equal.
   bool Equals(const Input& other) const;
 
   // Returns a pointer to the Input's data. This method is marked as "unsafe"
   // because access to the Input's data should be done through ByteReader
   // instead. This method should only be used where using a ByteReader truly
   // is not an option.
   const uint8_t* UnsafeData() const { return data_; }
 
   // Returns a copy of the data represented by this object as a std::string.
   std::string AsString() const;
 
+  // Returns a StringPiece pointing to the same data as the Input. The resulting
+  // StringPiece cannot outlive the data that was used to construct this Input.

[eroman, 2016/01/13 22:54:00]

nit: cannot --> must not

[nharper, 2016/01/13 23:04:48]

Done.

+  base::StringPiece AsStringPiece() const;
+
  private:
+  // This constructor is deleted to prevent constructing an Input from a
+  // std::string r-value. Since the Input points to memory owned by another
+  // object, such an Input would point to invalid memory. Without this deleted
+  // constructor, a std::string could be passed in to the base::StringPiece
+  // constructor because of StringPiece's implicit constructor.
+  Input(std::string) = delete;

[eroman, 2016/01/13 22:54:00]

Does this need to be const std::string& ?

[nharper, 2016/01/13 23:04:47]

I tested the following, and they all failed to compile:
Input test("test");
---
std::string t = "test";
Input test(t);
---
std::string TestString() { return "hello"; }
Input test(TestString())

What sort of things should I try that might slip past Input(std::string) but not
Input(const std::string&) ?

[eroman, 2016/01/13 23:21:06]

Sounds good. My only other test suggestions would be around the fact that it is
implicit ctor rather than explicit, but I don't anticipate problems there.

   const uint8_t* data_;
   size_t len_;
 };
 
 // Returns true if |lhs|'s data is lexicographically less than |rhs|'s data.
 NET_EXPORT_PRIVATE bool operator<(const Input& lhs, const Input& rhs);
 
 // This class provides ways to read data from an Input in a bounds-checked way.
 // The ByteReader is designed to read through the input sequentially. Once a
 // byte has been read with a ByteReader, the caller can't go back and re-read
@@ skipping 76 matching lines @@
   explicit Mark(const uint8_t* ptr);
   Mark();
   const uint8_t* ptr_;
 };
 
 }  // namespace der
 
 }  // namespace net
 
 #endif  // NET_DER_INPUT_H_