[PATCH] D35422: Add MemoryMappedSection struct for two-level memory map iteration

[Kostya Serebryany via Phabricator via llvm-commits]

kcc added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_procmaps.h:60
+
+#if SANITIZER_MAC
+
----------------
fjricci wrote:
> kcc wrote:
> > Folks, please avoid ifdefs like this. 
> > I'd appreciate if you can refactor the code to get rid of this ifdef (e.g move stuff to a separate file)
> This file already has quite a few OS-based #ifdefs in it (which is why I didn't think adding another would be problematic). Is it worth trying to refactor in a way that they all go away, or just this one in particular?
> already has quite a few OS-based #ifdefs

Not too many in fact. 
And when there are N bad things in a file it's not always an excuse to add (N+1)-th

> Is it worth trying to refactor in a way that they all go away

Yes, please. It's the same amount of work anyway.
E.g. declare a private object 
  MemoryMappingLayoutRep *rep
and implement MemoryMappingLayoutRep in OS-specific files. 


Repository:
  rL LLVM

https://reviews.llvm.org/D35422