[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