[libc-commits] [PATCH] D146001: [libc] enable printf using system FILE

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/stdio/fprintf.cpp:21
+#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+using file_t = __llvm_libc::File;
+#else  // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
----------------
Nit: Use `UpperCase` names for internal types.


================
Comment at: libc/src/stdio/printf.cpp:19
+#include "src/__support/File/file.h"
+static __llvm_libc::File *printf_stdout = __llvm_libc::stdout;
+#else  // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
----------------
Make this a macro, say `PRINTF_STDOUT`. Otherwise, you are adding a new symbol and an indirection.


================
Comment at: libc/src/stdio/printf_core/file_writer.h:89
+                                          size_t len) {
+  size_t written = ::fwrite(to_write, 1, len, file);
+  if (written != len || ::ferror(file))
----------------
You can call `flockfile` in the constructor and `funlockfile` in the destructor, and make this a call to `fwrite_unlocked`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146001