[libc-commits] [PATCH] D124421: [libc] add printf writer

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/stdio/printf_core/writer.h:31
+  // buffer it's writing to. This is because it doesn't handle the null
+  // terminator.
   size_t max_length;
----------------
sivachandra wrote:
> Can you explain the need for `max_length`? It seems to me like it is something a specific writer, say `StringWriter` should care about, but not a generic writer?
> 
> At this point, I would actually ask the need for a generic `Writer` class. It seems to me like all you need is a writer class which implements a method with a specific signature. For example, you want `StringWriter` like this:
> 
> ```
> class StringWriter {
> private:
>   ... // State as suggested in another comment
> public:
> 
>   // This method attempts to write |len| bytes from |data| and return the actual
>   // number of bytes written.
>   size_t write(const void *data, size_t len);
> };
> ```
> 
> FWIW, the `File` class already implements the `write` protocol. So, based on the printf function, `sprintf` or `fprintf`, one either uses the `StringWriter` or `File`.
Actually, I am beginning think there is benefit with the generic `Writer` indirection. Consider the `File` example again - file writer might want to explicitly lock and use unlocked file methods to write to the file. Having a generic writer allows us to hide such details in the file writer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124421