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

Mon Mar 20 11:53:09 PDT 2023

michaelrj added inline comments.

================
Comment at: libc/src/stdio/printf.cpp:31
+#ifndef LIBC_COPT_PRINTF_USE_PUBLIC_FILE
   int ret_val = printf_core::vfprintf_internal(
       reinterpret_cast<::FILE *>(__llvm_libc::stdout), format, args);
----------------
sivachandra wrote:
> michaelrj wrote:
> > sivachandra wrote:
> > > Have you considered making `vfprintf_internal` a template function which takes a suitable `FileWriter` object? The goal should be to confine `#ifdef` to only one place in the code. Since only one of the templates will get instantiated, it will not lead to code size bloat.
> > `vfprintf_internal` already doesn't have this `#ifdef`, this condition only changes where the stdout comes from. I've changed the code to clarify that.
> Sorry, my comment here is more general. Right now, you have a `#ifdef` conditional here, and two more in `file_writer.cpp` and `file_writer.h`. Using appropriate templates, you should be able to limit the `#ifdef` to just this file.
I can't limit it to just this file, since fprintf also needs to select the correct template, but it's just in the entrypoints now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146001