[PATCH] D59551: [scudo][standalone] Add error reports

Mitch Phillips via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 19 10:32:51 PDT 2019


hctim added inline comments.


================
Comment at: lib/scudo/standalone/report.cc:10
+#include "report.h"
+#include "atomic_helpers.h"
+#include "string_utils.h"
----------------
Nit: Newline between `{report.h}` and `{atomic_helpers.h, string_utils.h}`. Module header should be first, but putting the module header and subproject headers together without a newline makes them look like they should be in sorted order.


================
Comment at: lib/scudo/standalone/report.h:36
+void NORETURN reportOutOfMemory(uptr RequestedSize);
+enum : u8 {
+  Recycling,
----------------
Is there a reason why the use of this enum is deliberately occluded from those who use it? Why don't you use this as an `enum class AllocatorAction` with an internal `toString` method, and then `reportInvalidChunkState(..)` and others can take `AllocatorAction Action` as the first argument, instead of occluding it into a u8?

This would also remove the need to have `ActionsCount` and check-fail on this (`report.cc:115`), as you would have compile-time guarantees that the provided enum value is in range.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D59551





More information about the llvm-commits mailing list