Use base::StringPiece for input parameters in Encryptor, rather than std::string

crypto/encryptor.h

Index: crypto/encryptor.h
diff --git a/crypto/encryptor.h b/crypto/encryptor.h
index 712a19c78b8aedc1f3207573964bea09cb4b7bbf..cfab6f284b902291662d42172e3f451ca628d30d 100644
--- a/crypto/encryptor.h
+++ b/crypto/encryptor.h
@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
+#include "base/string_piece.h"
 #include "build/build_config.h"
 #include "crypto/crypto_api.h"
 
@@ -34,7 +35,7 @@ class CRYPTO_API Encryptor {
   // Only 128-bits counter is supported in this class.
   class Counter {
    public:
-    Counter(const std::string& counter);
+    Counter(const base::StringPiece& counter);
     ~Counter();
 
     // Increment the counter value.
@@ -61,19 +62,19 @@ class CRYPTO_API Encryptor {
   // key or the initialization vector cannot be used.
   //
   // When |mode| is CTR then |iv| should be empty.
-  bool Init(SymmetricKey* key, Mode mode, const std::string& iv);
+  bool Init(SymmetricKey* key, Mode mode, const base::StringPiece& iv);
 
   // Encrypts |plaintext| into |ciphertext|.
-  bool Encrypt(const std::string& plaintext, std::string* ciphertext);
+  bool Encrypt(const base::StringPiece& plaintext, std::string* ciphertext);
 
   // Decrypts |ciphertext| into |plaintext|.
-  bool Decrypt(const std::string& ciphertext, std::string* plaintext);
+  bool Decrypt(const base::StringPiece& ciphertext, std::string* plaintext);

[wtc, 2011/06/28 22:42:09]

It is strange to use StringPiece for input and std::string
for output in the Encrypt() and Decrypt() methods.

Can we not change these two methods?  Or can we also use
StringPiece for output?

This comment also applies to the private Crypt and CryptCTR
methods below.

[Ryan Sleevi, 2011/06/28 22:56:12]

No. base::StringPiece doesn't carry with it any ownership semantics - it's just
a pointer+size holder - the original object needs to stick around for the
duration of the StringPiece.

As a result, it's not possible to transfer ownership of a new pointer to the
caller. This would be equivalent to the signature
Encrypt(T* input_pointer, scoped_refptr<T>* output_pointer)

Unfortunately, we can't do something like StringPiece* because whatever the
StringPiece pointed to would be out of scope when the function returned.

The semantics could be changed if the caller was guaranteed to give a properly
sized StringPiece, we could write into it, but I think that's leaky on several
levels - most importantly, being that it requires the callers to know about
padding and block sizes, which I think would be a poor API.

 
   // Sets the counter value when in CTR mode. Currently only 128-bits
   // counter value is supported.
   //
   // Returns true only if update was successful.
-  bool SetCounter(const std::string& counter);
+  bool SetCounter(const base::StringPiece& counter);
 
   // TODO(albertb): Support streaming encryption.
 
@@ -107,21 +108,21 @@ class CRYPTO_API Encryptor {
 
 #if defined(USE_OPENSSL)
   bool Crypt(bool encrypt,  // Pass true to encrypt, false to decrypt.
-             const std::string& input,
+             const base::StringPiece& input,
              std::string* output);
   std::string iv_;
 #elif defined(USE_NSS)
   bool Crypt(PK11Context* context,
-             const std::string& input,
+             const base::StringPiece& input,
              std::string* output);
   bool CryptCTR(PK11Context* context,
-                const std::string& input,
+                const base::StringPiece& input,
                 std::string* output);
   ScopedPK11Slot slot_;
   ScopedSECItem param_;
 #elif defined(OS_MACOSX)
   bool Crypt(int /*CCOperation*/ op,
-             const std::string& input,
+             const base::StringPiece& input,
              std::string* output);
 
   std::string iv_;