[PATCH] D38098: Removed platform-specific ifdefs from sanitizer_procmaps.h

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added a comment.

LGTM with few nits



================
Comment at: lib/sanitizer_common/sanitizer_linux.h:34
+    uptr mmaped_size;
+    uptr len;
+  };
----------------
please fix formating, 
e.g with git clang-format --style file -f HEAD~1


================
Comment at: lib/sanitizer_common/sanitizer_procmaps.h:19
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_mutex.h"
+#include "sanitizer_mac.h"
----------------
order please


================
Comment at: lib/sanitizer_common/sanitizer_procmaps.h:25
 
-#if SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD
-struct ProcSelfMapsBuff {
-  char *data;
-  uptr mmaped_size;
-  uptr len;
-};
-
-// Reads process memory map in an OS-specific way.
-void ReadProcMaps(ProcSelfMapsBuff *proc_maps);
-#endif  // SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD
+struct MemoryMappingLayoutData;
 
----------------
you don'e need this forward declaration as it's already included just above.


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_common.cc:20
 #include "sanitizer_procmaps.h"
+#include "sanitizer_linux.h"
 
----------------
unneeded include?


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_freebsd.cc:16
 #include "sanitizer_common.h"
+#include "sanitizer_linux.h"
 #if SANITIZER_FREEBSD
----------------
unneeded include?


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_linux.cc:17
 #include "sanitizer_procmaps.h"
+#include "sanitizer_linux.h"
 
----------------
already included with sanitizer_procmaps.h


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_mac.cc:48
 };
-
 template <typename Section>
----------------
please don't make such changes unrelated code.


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_mac.cc:185
+static bool NextSegmentLoad
+(MemoryMappedSegment *segment, MemoryMappedSegmentData *seg_data,
+MemoryMappingLayoutData *layout_data) {
----------------
argument "MemoryMappedSegmentData *seg_data" is not needed


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_mac.cc:192
     uptr base_virt_addr, addr_mask;
-    if (current_image_ == kDyldImageIdx) {
+    CHECK(layout_data);
+    if (layout_data->current_image == kDyldImageIdx) {
----------------
no point to CHECK here


https://reviews.llvm.org/D38098