[PATCH] D61326: Fixes for builds that require strict X/Open and POSIX compatiblity

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast requested changes to this revision.
hubert.reinterpretcast added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: jsji.

> These are defined under POSIX

Do you mean that these aren't defined under POSIX?



================
Comment at: llvm/lib/Support/Unix/Memory.inc:94
 
-  int fd = -1;
+  // on platforms that have it we can use MAP_ANONYMOUS to get a memory mapped
+  // page without file backing, but we need a fallback by opening /dev/zero
----------------
<ins>On</ins><del>on</del> platforms that have it<ins>,</ins> we can use `MAP_ANONYMOUS` to get a memory<ins>-</ins>mapped page without file backing, but we need a fallback <del>by</del><ins>of</ins> opening `/dev/zero` for strictly POSIX platforms instead<ins>.</ins>


================
Comment at: llvm/lib/Support/Unix/Memory.inc:97
+  // for strictly POSIX platforms instead
+  int fd =
+#if defined(MAP_ANONYMOUS) || defined(MAP_ANON)
----------------
I would rather have `fd` assigned separately on each branch of the `#if` than to have the rest of the statement inside conditional inclusion blocks.


================
Comment at: llvm/lib/Support/Unix/Memory.inc:102
+  open("/dev/zero", O_RDWR);
+  if (fd ==-1) {
+    EC = std::error_code(errno, std::generic_category());
----------------
Add a space after `==`.


================
Comment at: llvm/lib/Support/Unix/Memory.inc:112
+  | MAP_ANONYMOUS
+#elif MAP_ANON
+  | MAP_ANON
----------------
To be consistent with the `ifdef`, this should be `#elif defined(MAP_ANON)`.


================
Comment at: llvm/lib/Support/Unix/Memory.inc:135
     if (NearBlock) //Try again without a near hint
       return allocateMappedMemory(NumBytes, nullptr, PFlags, EC);
 
----------------
The file descriptor returned by `open` and stored in `fd` is not closed on this path.


================
Comment at: llvm/lib/Support/Unix/Path.inc:157
     return nullptr;
   s = pv = strdup(pv);
   if (!pv)
----------------
Once assigned, `s` always has the same value as `pv`. Do we need `s`?


================
Comment at: llvm/lib/Support/Unix/Path.inc:161
+  char *state;
+  if ((t = strtok_r(s,":",&state)) != nullptr) {
+    do {
----------------
Add spaces after the commas in the argument list.


================
Comment at: llvm/lib/Support/Unix/Path.inc:167
+      }
+    } while((t = strtok_r(nullptr,":",&state)) != nullptr);
   }
----------------
Add spaces after the commas in the argument list.
Add space after `while` (or at least that is what `clang-format` does for me).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61326/new/

https://reviews.llvm.org/D61326