[libc-commits] [PATCH] D122773: [libc][NFC] add outline of printf

Michael Jones via Phabricator via libc-commits libc-commits at lists.llvm.org
Thu Mar 31 16:19:48 PDT 2022

michaelrj added inline comments.

Comment at: libc/src/stdio/printf_files/writer.h:29
+  size_t max_length;
+  size_t chars_written;
sivachandra wrote:
> What is `max_length`?
for `snprintf` there is a limit on the maximum number of characters that can be written. This represents that maximum length. In other cases it will be set to a default (probably size_t maximum).

Comment at: libc/src/stdio/printf_files/writer.h:38
+  // written. It always increments chars_written by length.
+  void write(const char *new_string, size_t length);
sivachandra wrote:
> These functions should be implemented inline to avoid multiple indirections?
Could you clarify what you mean by implementing this inline? This function won't just be calling the raw_write function. If length + chars_written > max_length then the length passed to raw_write will be max_length - chars_written, so that this doesn't overrun the buffer.

Comment at: libc/src/stdio/printf_files/writer.h:20
+class Writer {
+  void *output;
sivachandra wrote:
> michaelrj wrote:
> > sivachandra wrote:
> > > Is this class envisioned to be a `final` class? If yes, then we should have documentation of the expectations. For example, what is `output`, `raw_write` etc. If not also, we should add documentation about how it is to be specialized.
> > I was under the impression that virtual functions were disallowed in libc, so all classes were implicitly final.
> > 
> > I've added comments with more explanation on how this specialization works.
> Couple of things:
> 1. A class can be final without a virtual function.
> 2. Going by your comments, this is class is a wrapper over the actual write streams. Which makes me feel it is a final class (as in, the intention is not to specialize it.) You don't need to mark it final though.
I've marked it as final for clarity.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list