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