dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land.
This LGTM with a couple of nitpicks. ================ Comment at: compiler-rt/lib/builtins/os_version_check.c:183 CFStringRef ProductVersion = (*CFStringCreateWithCStringNoCopyFunc)( - NULL, "ProductVersion", kCFStringEncodingASCII, kCFAllocatorNull); + NULL, "ProductVersion", CF_STRING_ENCODING_ASCII, kCFAllocatorNull); if (!ProductVersion) ---------------- It wasn't obvious that `kCFAllocatorNull` had been `dlsym`'ed in. Is there a way of naming this that would make it more clear you weren't getting this from an `#include`? Or does it not matter? ================ Comment at: compiler-rt/lib/builtins/os_version_check.c:211-214 + if (Major < GlobalMajor) return 1; + if (Major > GlobalMajor) return 0; + if (Minor < GlobalMinor) return 1; + if (Minor > GlobalMinor) return 0; ---------------- If you're going to do this, please do it in a separate NFC commit since it appears to be whitespace only. Repository: rCRT Compiler Runtime https://reviews.llvm.org/D50269 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
