Removed platform-specific ifdefs for linux, mac, freebsd and netbsd from sanitizer_procmaps.h
Details
Diff Detail
- Build Status
Buildable 10471 Build 10471: arc lint + arc unit
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 | ||
---|---|---|
18 | This is linux file. It can contain, even in future, things incompatible with FREEBSD |
lib/sanitizer_common/sanitizer_procmaps_freebsd.cc | ||
---|---|---|
18 | 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 | ||
---|---|---|
82 | So do you think I should create these two files? |
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
82 | 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 | ||
---|---|---|
82 | Haha, alright, I also like that idea |
lib/sanitizer_common/sanitizer_procmaps.h | ||
---|---|---|
82 | sanitizer_mac.h and sanitizer_linux.h are good as well |
LGTM with few nits
lib/sanitizer_common/sanitizer_linux.h | ||
---|---|---|
34 | please fix formating, | |
lib/sanitizer_common/sanitizer_procmaps.h | ||
19 | order please | |
23 | you don'e need this forward declaration as it's already included just above. | |
lib/sanitizer_common/sanitizer_procmaps_common.cc | ||
20 | unneeded include? | |
lib/sanitizer_common/sanitizer_procmaps_freebsd.cc | ||
16 | unneeded include? | |
lib/sanitizer_common/sanitizer_procmaps_linux.cc | ||
17 | already included with sanitizer_procmaps.h | |
lib/sanitizer_common/sanitizer_procmaps_mac.cc | ||
48 | please don't make such changes unrelated code. | |
200 | argument "MemoryMappedSegmentData *seg_data" is not needed | |
207–208 | 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 | ||
---|---|---|
23 | what about this one? |
LGTM with one nit.
lib/sanitizer_common/sanitizer_procmaps_linux.cc | ||
---|---|---|
16 | This shouldn't be needed anymore. |
please fix formating,
e.g with git clang-format --style file -f HEAD~1