Removed platform-specific ifdefs for linux, mac, freebsd and netbsd from sanitizer_procmaps.h
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
title/description are misleading. I don't see anything related to ifdefs
Why do you need to change this, it seems right now it's were it should be.
lib/sanitizer_common/sanitizer_procmaps_freebsd.cc | ||
---|---|---|
17 ↗ | (On Diff #116065) | This is linux file. It can contain, even in future, things incompatible with FREEBSD |
lib/sanitizer_common/sanitizer_procmaps_freebsd.cc | ||
---|---|---|
17 ↗ | (On Diff #116065) | Meh, it's called linux, but already has SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD |
We're working on getting some git issues sorted out, this isn't currently the full commit
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
89 ↗ | (On Diff #116079) | So do you think I should create these two files? |
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
89 ↗ | (On Diff #116079) | I would lean towards just using sanitizer_mac.h, like you already did for sanitizer_linux.h, since it seems silly to create a header for just one struct. But will defer to @vitalybuka |
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
89 ↗ | (On Diff #116079) | Haha, alright, I also like that idea |
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
89 ↗ | (On Diff #116079) | sanitizer_mac.h and sanitizer_linux.h are good as well |
LGTM with few nits
lib/sanitizer_common/sanitizer_linux.h | ||
---|---|---|
34 ↗ | (On Diff #116222) | please fix formating, |
lib/sanitizer_common/sanitizer_procmaps.h | ||
19 ↗ | (On Diff #116222) | order please |
25 ↗ | (On Diff #116222) | you don'e need this forward declaration as it's already included just above. |
lib/sanitizer_common/sanitizer_procmaps_common.cc | ||
20 ↗ | (On Diff #116222) | unneeded include? |
lib/sanitizer_common/sanitizer_procmaps_freebsd.cc | ||
16 ↗ | (On Diff #116222) | unneeded include? |
lib/sanitizer_common/sanitizer_procmaps_linux.cc | ||
17 ↗ | (On Diff #116222) | already included with sanitizer_procmaps.h |
lib/sanitizer_common/sanitizer_procmaps_mac.cc | ||
48 ↗ | (On Diff #116222) | please don't make such changes unrelated code. |
185 ↗ | (On Diff #116222) | argument "MemoryMappedSegmentData *seg_data" is not needed |
192 ↗ | (On Diff #116222) | no point to CHECK here |
@krytarowski - how important is it for you to pre-test this on NetBSD? Does NetBSD run on buildbots? Just asking because Tuesday is a ways off, and this change seems fairly unlikely to break things given that it passes darwin/linux tests locally and is more or less NFC.
I think the argument *seg_data in NextSegmentLoad is still necessary, since it is a private member and the static function has no access to it
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
25 ↗ | (On Diff #116222) | what about this one? |
LGTM with one nit.
lib/sanitizer_common/sanitizer_procmaps_linux.cc | ||
---|---|---|
16 ↗ | (On Diff #116254) | This shouldn't be needed anymore. |