[PATCH] D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available

[Alex Lorenz via Phabricator via llvm-commits]

arphaman added a comment.

We need to add a test for this function as well, can you try to create one in test/builtins/Unit?



================
Comment at: lib/builtins/os_version_check.c:31
+/* Find and parse the SystemVersion.plist file. */
+static void ParseSystemVersionPList(void *Unused) {
+  (void)Unused;
----------------
NIT: The function name can be lowercase


================
Comment at: lib/builtins/os_version_check.c:61
+
+  PListBuf = malloc((size_t)PListFileSize);
+  size_t NumRead = fread(PListBuf, 1, (size_t)PListFileSize, PropertyList);
----------------
Please add a check that verifies that malloc succeeded.


================
Comment at: lib/builtins/os_version_check.c:81
+  char VersionStr[32];
+  CFStringGetCString((CFStringRef)OpaqueValue, VersionStr, sizeof(VersionStr),
+                     kCFStringEncodingUTF8);
----------------
Please add a check to see if this call fails.


================
Comment at: lib/builtins/os_version_check.c:94
+
+int32_t _IsOSVersionAtLeast(int32_t Major, int32_t Minor, int32_t Subminor) {
+  /* Populate the global version variables, if they haven't already. */
----------------
The function should be renamed to match the new name emitted by clang.


https://reviews.llvm.org/D30136