[libc-commits] [PATCH] D126831: [libc] add printf

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Fri Jun 10 12:12:06 PDT 2022

sivachandra added inline comments.

Comment at: libc/src/stdio/printf.cpp:33
+  __llvm_libc::stdout->lock();
   int ret_val = printf_core::printf_main(&writer, format, args);
Nit: There is an RAII file lock:

__llvm_libc::File::FileLock lock(__llvm_libc::stdout);

Couple of things more:
1. It seems to me like this is not the place to do the lock/unlock and call to error_unlocked. It should be managed by the file writer and the errors should be surfaced to printf_main. See another comment below.
2. `printf` and `fprintf` should share code in some manner instead of being mostly duplicated.

Comment at: libc/src/stdio/printf_core/writer.h:41
+  // negative value.
   void write(const char *new_string, size_t length);
If this method returns an int value, which can be negative on error, printf_main can do an early return as soon as it sees an error. If this is OK, you should do the same with the converter functions. Why is that not a preferred design?

  rG LLVM Github Monorepo



More information about the libc-commits mailing list