[PATCH] D59116: [scudo][standalone] Implement checksumming functions

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/checksum.cc:68
+}
+#endif
+
----------------
Perhaps comments with the matching ifdefs would be helpful here, especially since this last one matches way up at line 27.


================
Comment at: lib/scudo/standalone/checksum.h:10
+#ifndef SCUDO_CRC32_H_
+#define SCUDO_CRC32_H_
+
----------------
`SCUDO_CHECKSUM_H`


================
Comment at: lib/scudo/standalone/checksum.h:20
+
+#if defined(__SSE4_2__) || defined(__ARM_FEATURE_CRC32)
+#ifdef __SSE4_2__
----------------
The outer if-defined looks unnecessary.


================
Comment at: lib/scudo/standalone/checksum.h:38
+
+// That checksum, unlike a software CRC32, doesn't use any array lookup. We save
+// significantly on memory accesses, as well as 1K of CRC32 table, on platforms
----------------
s/That/BSD


================
Comment at: lib/scudo/standalone/checksum.h:45
+    Sum = static_cast<u16>((Sum >> 1) | ((Sum & 1) << 15));
+    Sum = static_cast<u16>(Sum + (Data & 0xff));
+    Data >>= 8;
----------------
Are these `static_casts` necessary?


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59116/new/

https://reviews.llvm.org/D59116