[PATCH] D90483: [GWP-ASan] Fuchsia specific mapping & utilities functions

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added inline comments.


================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_fuchsia.cpp:36
+                        0, Vmo, 0, Size, &Addr);
+  Check(Status == ZX_OK, "Vmo mapping failed");
+  _zx_handle_close(Vmo);
----------------
cryptoad wrote:
> mcgrathr wrote:
> > Is it possible for the failure message to include `Status` or `zx_status_get_string(Status)`?
> I'd probably take that as an improvement for later as current check & die only take a string and there is no string formatting function in GWP-ASan.
Understood.


================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/guarded_pool_allocator_fuchsia.h:9
+
+#if defined(__Fuchsia__)
+#ifndef GWP_ASAN_GUARDED_POOL_ALLOCATOR_FUCHSIA_H_
----------------
cryptoad wrote:
> mcgrathr wrote:
> > It seems weird to have any `#if` outside the header guard.  Obviously inverting the order makes no practical difference but I think it makes the code more readable to have it inside and separated by a blank line.
> Right, that was initially the way I went, but in the previous review, Mitch suggested the other way.
> I can go one way or the other if we have a consensus.
On all style/cosmetic points I defer to the primary maintainer.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90483