[libc-commits] [PATCH] D71094: [libc] Add implementation of errno.

[Alex Brachet via Phabricator via libc-commits]

abrachet accepted this revision.
abrachet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/config/linux/api.td:33
+  let Defn = [{
+    extern int *__errno_location();
+    #define errno *__errno_location()
----------------
We don't use `extern` for other function definitions in other header files so I don't think we should here


================
Comment at: libc/spec/stdc.td:176
+      [
+        Macro<"errno">,
+      ],
----------------
Are adding the errno macros out of the scope of this patch?


================
Comment at: libc/src/errno/errno_test.cpp:13
+
+TEST(ErrnoTest, Smoke) {
+  int test_val = 123;
----------------
Something like `Basic` seems more conventional than `Smoke` for a test name. What is it supposed to mean? 


================
Comment at: libc/src/errno/llvmlibc_errno.h:12
+
+// Internal code should use this macro and not use the errno macro from the
+// public header.
----------------
Internally we could use a `thread_local int &` instead of relying on a macro, I believe. It seems nicer, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71094