[libc-commits] [PATCH] D78184: [libc] Add write(2) implementation for Linux and FDReader test utility

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra marked 2 inline comments as done.
sivachandra added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/config/linux/api.td:308
+
+def UniStdAPI : PublicAPI<"unistd.h"> {
+  let Functions = [
----------------
abrachet wrote:
> sivachandra wrote:
> > I think `write` is part of the POSIX standard. So, we should prefer to put this in the wider standard.
> This is in the linux/api.td. We don't currently have a way to separate the common definitions into a common record files. The HeaderSpec is in posix.td though.
Sorry, I am not sure what I was thinking then.


================
Comment at: libc/spec/posix.td:185
+          "write",
+          RetValSpec<SizeTType>, // TODO: this should be ssize_t from sys/types.h
+          [ArgSpec<IntType>, ArgSpec<ConstVoidPtr>, ArgSpec<SizeTType>]
----------------
abrachet wrote:
> sivachandra wrote:
> > The POSIX standard says that `unistd.h` should define the `ssize_t` type. I think you should add the definition in this patch and use it. Since the standard expects multiple header files to define this type, you should put it in: https://github.com/llvm/llvm-project/blob/master/libc/include/__posix-types.h and "expose" the definition via `unistd.h`.
> Done. I think we will want to generate `__posix-types.h` eventually.
Agreed.


================
Comment at: libc/test/src/unistd/write_test.cpp:19
+  __llvm_libc::testutils::FDReader reader;
+  EXPECT_THAT(__llvm_libc::write(reader.getWriteFD(), "hello", 5), Succeeds(5));
+  EXPECT_TRUE(reader.matchWritten("hello"));
----------------
Nitty nit: May be create a var for `"hello"`.


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

https://reviews.llvm.org/D78184