[PATCH] D35554: Add NetBSD support in sanitizer_platform_limits_posix.*

Filipe Cabecinhas via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 19 21:34:26 PDT 2017


filcab added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.cc:296
+#define SHM_STAT -1
+  unsigned struct_shm_info_sz = -1;
+#endif
----------------
Setting these to `-1` looks weird. Can you at least add a small comment? I'm assuming these simply don't exist on NetBSD.


================
Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.h:124
 #if SANITIZER_LINUX || SANITIZER_FREEBSD
-
 #if defined(__powerpc64__) || defined(__s390__)
----------------
Stray change.


================
Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.h:182
+    u32 aio_buf;
+    u32 aio_nbytes;
+    u32 aio_fildes;
----------------
Maybe use `uptr` (Or `char*`) for `aio_buf` and `long` for `aio_nbytes` and others below?
It seems you can merge these two platforms.


================
Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.h:191
+#else
+#error port this to your platform
+#endif
----------------
There's more than one of these. Right now you're just dealing with x86 and x86_64, so why not just have one `#error` at the start of the file if you need to, and then not have these cases?


================
Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.h:982
+  };
+#define SANITIZER_HAS_STRUCT_FILE 0  // not ported
 #else
----------------
If this is set to 0, can't you just use the `#else` case, which defined `__sanitizer_FILE` to `void`?


Repository:
  rL LLVM

https://reviews.llvm.org/D35554





More information about the llvm-commits mailing list