[libc-commits] [PATCH] D126329: [libc] Add global stdout and stderr objects.

[Siva Chandra via Phabricator via libc-commits]

sivachandra marked an inline comment as done.
sivachandra added inline comments.


================
Comment at: libc/src/__support/File/linux_file.cpp:169-170
+constexpr size_t STDOUT_BUFFER_SIZE = 1024;
+char stdout_buffer[STDOUT_BUFFER_SIZE];
+LinuxFile StdOut(1, stdout_buffer, STDOUT_BUFFER_SIZE, 0, false,
+                 File::ModeFlags(File::OpenMode::APPEND));
----------------
abrachet wrote:
> These should probably be static
Yes, fixed!


================
Comment at: libc/src/__support/File/linux_file.cpp:176
+char stderr_buffer[STDOUT_BUFFER_SIZE];
+LinuxFile StdErr(1, stderr_buffer, STDERR_BUFFER_SIZE, 0, false,
+                 File::ModeFlags(File::OpenMode::WRITE));
----------------
abrachet wrote:
> I don't know what values are ok for `buffer_mode` but stderr would logically have a different buffer mode from stdout because of no line buffering.
> 
> Why does stderr have `OpenMode::WRITE` and stdout has `OpenMode::APPEND`
About buffering mode, we do not have line buffering support yet so I just left the value at 0. I have added a TODO to update that when we add line buffering support.

About `WRITE` vs `APPEND`, My intention was to put `APPEND` on both but I was playing with `WRITE` on one of them but forgot to put it back to `APPEND`. I have updated both to `APPEND` now.

Overall, this is a total case of copy-paste oversight. Sorry about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126329