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

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Nit: You should probably keep the addition of `vfprintf_internal` in the other patch.



================
Comment at: libc/src/stdio/printf_core/char_converter.h:22
+  int result = 0;
+#define RET_IF_RESULT_NEGATIVE()                                               \
+  if (result < 0)                                                              \
----------------
Nit: If you make this macro take an argument, you can use like this:

```
RETURN_ON_ERROR(writer->write(...))
```


================
Comment at: libc/src/stdio/printf_core/writer.cpp:21
+  if (result < 0) {
+    chars_written = result;
+  } else if (chars_written >= 0) {
----------------
Is this required now?


================
Comment at: libc/src/stdio/printf_core/writer.h:32
 
-  unsigned long long chars_written = 0;
+  int chars_written = 0;
 
----------------
Do we need this to be a signed integer now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127773