[PATCH] D58184: [scudo][standalone] Introduce platform specific code & mutexes

Matt Morehouse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 19 13:01:09 PST 2019


morehouse added inline comments.


================
Comment at: lib/scudo/standalone/mutex.h:57
+  SpinMutex(const SpinMutex &);
+  void operator=(const SpinMutex &);
+};
----------------
cryptoad wrote:
> morehouse wrote:
> > Are these constructors ever used?  If not, maybe just use `SpinMutex(const SpinMutex &) = delete`.
> I will change that.
I think `operator=` needs to be deleted as well to avoid copies.  Below as well.


================
Comment at: lib/scudo/standalone/tests/map_test.cc:51
+  const scudo::uptr Size = 4 * PageSize;
+  scudo::u64 PlatformData = 0;
+  void *P = scudo::map(nullptr, Size, MappingName, MAP_NOACCESS, &PlatformData);
----------------
This boilerplate would be a good candidate to put in a test setup class.


================
Comment at: lib/scudo/standalone/tests/map_test.cc:64
+
+// Death tests
+
----------------
Maybe we should combine these with the above tests since they will only cost ~1 line each then. 


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D58184





More information about the llvm-commits mailing list