[libc-commits] [PATCH] D155766: [libc] Remove global constructor in `getopt` implementation

Alex Brachet via Phabricator via libc-commits libc-commits at lists.llvm.org
Thu Jul 20 05:47:03 PDT 2023


abrachet accepted this revision.
abrachet added a comment.
This revision is now accepted and ready to land.

In D155766#4518486 <https://reviews.llvm.org/D155766#4518486>, @jhuber6 wrote:

> The `__llvm_libc::stderr` depends on the address of a value that cannot be statically determined, see https://godbolt.org/z/WYjeq3bnK, so it's like you say with the relocations.  I don't think there's a way to make this a constant expression without the `File` API exporting the underlying static variable here <https://github.com/llvm/llvm-project/blob/2c38740ca661a10866a796db105752e15372ddce/libc/src/__support/File/linux/file.cpp#L173>.

Oh right I see now, we aren't taking the address of `__llvm_libc::stderr`. I guess then we could hold a `FILE**` instead and do `fprintf(*errstream, ...)` or I'm fine with the change as is too.



================
Comment at: libc/src/unistd/getopt.cpp:46-50
+      if (errstream)
+        __llvm_libc::fprintf(errstream, fmt, ts...);
+      else
+        __llvm_libc::fprintf(reinterpret_cast<FILE *>(__llvm_libc::stderr), fmt,
+                             ts...);
----------------
I think this or something similar is cleaner than conditionals, but up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155766



More information about the libc-commits mailing list