[PATCH] D60787: [scudo][standalone] Introduce the Secondary allocator

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/scudo/standalone/fuchsia.h:20
+
+typedef struct PlatformData {
+  zx_handle_t Vmar;
----------------
Now I am thinking that name PlatformData in top namespace is not very helpful.
MmapPlatformData?


================
Comment at: lib/scudo/standalone/linux.h:20
+typedef struct PlatformData {
+  u8 Unused;
+} PlatformData;
----------------
Why this can't be empty?


================
Comment at: lib/scudo/standalone/secondary.h:66
+  // larger (greater than a page) alignments.
+  NOINLINE void *allocate(uptr Size, uptr AlignmentHint = 0,
+                          uptr *BlockEnd = nullptr) {
----------------
It's NOINLINE so can go into cpp file?


================
Comment at: lib/scudo/standalone/secondary.h:186
+
+  void disable() { Mutex.lock(); }
+
----------------
Why not just lock, unlock?


================
Comment at: lib/scudo/standalone/secondary.h:190
+
+  template <typename F> void iterateOverBlocks(F Callback) const {
+    for (LargeBlock::Header *H = Tail; H != nullptr; H = H->Prev)
----------------
Wouldn't be better if we lock inside of iterateOverBlocks


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D60787