blobhash: do not use cast for unaligned access

* remove the uncorrect comment. as std::hash<> does not apply to a
  customized type. see https://en.cppreference.com/w/cpp/utility/hash
* do not use cast for accessing an uint32_t by dereferencing (void *)
  or (char *) pointer. because the alignment requirement of `uint32_t`
  is stricter than that of `void*` or `char *`. we need to do an
  explicit memcpy() for accessing the uint32_t pointed by the pointer.
  see also https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt
* instead of using a loop for mixing the last few bytes. use a switch-
  case. so GCC-9 won't complain with
```
../src/include/blobhash.h:38:10: warning: iteration 4 invokes undefined
behavior [-Waggressive-loop-optimizations]
   38 |       sh += 8;
      |       ~~~^~~~
../src/include/blobhash.h:36:12: note: within this loop
   36 |     while (len) {
      |            ^~~
```
  at seeing `sh += 8;`
* instead of mixing the last bits repeatly by derefencing `p` using
  `uint32_t`, while move it forwards with step size of `1`, mix
  the bits byte-wise.
* include <cstdint>. this header file is supposed to be self-contained.
* use `std::uint32_t` instead of using `uint32_t`. we cannot assume
  somebody is `using namespace std` or `using std::uint32_t` for us.
* mark the operator() `const noexcept`. see
  https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c89-make-a-hash-noexcept
  for the rationale behind this

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/include/blobhash.h b/src/include/blobhash.h
index 597884e4c9d82e706dc453aa992049c0becda5da..97f2065ce3a621d031b96f439f001e132804349d 100644 (file)
--- a/src/include/blobhash.h
+++ b/src/include/blobhash.h
@@ -14,33 +14,40 @@
 #ifndef CEPH_BLOBHASH_H
 #define CEPH_BLOBHASH_H
 
+#include <cstdint>
 #include "hash.h"
 
-/*
-- this is to make some of the STL types work with 64 bit values, string hash keys, etc.
-- added when i was using an old STL.. maybe try taking these out and see if things 
-  compile now?
-*/
-
 class blobhash {
 public:
-  uint32_t operator()(const char *p, unsigned len) {
-    static rjhash<uint32_t> H;
-    uint32_t acc = 0;
+  uint32_t operator()(const void* p, size_t len) {
+    static rjhash<std::uint32_t> H;
+    std::uint32_t acc = 0;
+    auto buf = static_cast<const unsigned char*>(p);
     while (len >= sizeof(acc)) {
-      acc ^= *(uint32_t*)p;
-      p += sizeof(uint32_t);
-      len -= sizeof(uint32_t);
+      acc ^= unaligned_load(buf);
+      buf += sizeof(std::uint32_t);
+      len -= sizeof(std::uint32_t);
     }
-    int sh = 0;
-    while (len) {
-      acc ^= (uint32_t)*p << sh;
-      sh += 8;
-      len--;
-      p++;
+    // handle the last few bytes of p[-(len % 4):]
+    switch (len) {
+    case 3:
+      acc ^= buf[2] << 16;
+      [[fallthrough]];
+    case 2:
+      acc ^= buf[1] << 8;
+      [[fallthrough]];
+    case 1:
+      acc ^= buf[0];
+      [[fallthrough]];
     }
     return H(acc);
   }
+private:
+  static inline std::uint32_t unaligned_load(const unsigned char* p) {
+    std::uint32_t result;
+    __builtin_memcpy(&result, p, sizeof(result));
+    return result;
+  }
 };
 
 

diff --git a/src/msg/msg_types.h b/src/msg/msg_types.h
index d9ca27c3aec3f72cd130215ee499351c036f40b0..7557c51a82f97cdfa0b059feb5b6530795f024f4 100644 (file)
--- a/src/msg/msg_types.h
+++ b/src/msg/msg_types.h
@@ -565,7 +565,7 @@ namespace std {
 template<> struct hash<entity_addr_t> {
   size_t operator()( const entity_addr_t& x ) const {
     static blobhash H;
-    return H((const char*)&x, sizeof(x));
+    return H(&x, sizeof(x));
   }
 };
 } // namespace std