[libc-commits] [PATCH] D127773: [libc] refactor printf file writing

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/stdio/printf_core/file_writer.h:24
+public:
+  FileWriter(::FILE *init_file) {
+    file = reinterpret_cast<__llvm_libc::File *>(init_file);
----------------
sivachandra wrote:
> Something I realized while reviewing the other patch: why should this internal class take a `FILE*` arg only to cast to the internal file data structure?
we have to cast it somewhere, and doing it inside `vfprintf_internal` means that we don't have to cast it in both `fprintf` and `printf` (and potentially also the `v` variants of those functions if those are added).


================
Comment at: libc/src/stdio/printf_core/writer.h:32
 
-  unsigned long long chars_written = 0;
+  int chars_written = 0;
 
----------------
sivachandra wrote:
> Do we need this to be a signed integer now?
It doesn't need to be an int, but it does make sense. Printf always returns an int, and that int is supposed to be the number of characters written, so storing the number of characters written in anything bigger doesn't really help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127773