Moved SuperFastHash function from base/hash.cc to base/third_party.

base/hash.h

Index: base/hash.h
diff --git a/base/hash.h b/base/hash.h
index cf8ea3a26e49475a9187d7005cc17b0f470609f6..2a4c2ccdb59b8a8d6cb8a9c719384e85b4224955 100644
--- a/base/hash.h
+++ b/base/hash.h
@@ -10,11 +10,21 @@
 #include "base/base_export.h"
 #include "base/basictypes.h"
 
+#ifdef __cplusplus

[rvargas (doing something else), 2014/02/27 04:11:20]

Do we need this?

[Matt Giuca, 2014/02/27 04:24:09]

Assuming you mean #ifdef __cplusplus as opposed to extern "C" as a whole. You're
right, I should assume C++ in a C++ source file!

(Also, I just realised it shouldn't be in hash.h, but hash.cc -- whoops.)

+extern "C" {
+#endif
+
+// Definition in base/third_party/superfasthash/superfasthash.c. (Third-party
+// code did not come with its own header file, so declaring the function here.)
+uint32_t SuperFastHash(const char* data, int len);
+
+#ifdef __cplusplus
+}
+#endif
+
 namespace base {
 

[jln (very slow on Chromium), 2014/02/27 04:37:17]

Please, make it very clear that none of these should be used a cryptographically
secure hash functions.

[Matt Giuca, 2014/02/27 05:25:29]

Done.

-// From http://www.azillionmonkeys.com/qed/hash.html
-// This is the hash used on WebCore/platform/stringhash
-BASE_EXPORT uint32 SuperFastHash(const char * data, int len);
+BASE_EXPORT uint32 SuperFastHash(const char* data, int len);
 
 inline uint32 Hash(const char* key, size_t length) {
   return SuperFastHash(key, static_cast<int>(length));

[jln (very slow on Chromium), 2014/02/27 04:37:17]

This cast is implementation defined for values strictly larger than MAX_INT.
In this particular case, it won't lead to memory corruption iff SuperFastHash()
handles negative length with no memory access.

It's not great to have to rely on a dependency in third_party to respect an
implicit contract like this.

Could you please add:

if (length > std::numeric_limits<int>::max()) {
  NOTREACHED();
  return 0;
}

[Matt Giuca, 2014/02/27 05:25:29]

Done.

(Note that the existing implementation does check len <= 0, so there is no
existing security vulnerability here, but I agree that we should be checking all
the same.)