This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add write(2) implementation for Linux and FDReader test utility
ClosedPublic

Authored by abrachet on Apr 15 2020, 1:58 AM.

Details

Summary

Adds write for Linux and FDReader utility which should be useful for some stdio tests as well.

Diff Detail

Event Timeline

abrachet created this revision.Apr 15 2020, 1:58 AM

I have not read the FDReader implementation, but the rest of the change LG. I have couple comments though. I will do a final review once they are addressed.

libc/config/linux/api.td
319

I think write is part of the POSIX standard. So, we should prefer to put this in the wider standard.

libc/spec/posix.td
186

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.

abrachet updated this revision to Diff 258254.Apr 17 2020, 1:44 AM

Add ssize_t to __posix-types.h

abrachet marked 4 inline comments as done.Apr 17 2020, 1:47 AM
abrachet added inline comments.
libc/config/linux/api.td
319

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.

libc/spec/posix.td
186

Done. I think we will want to generate __posix-types.h eventually.

sivachandra accepted this revision.Apr 17 2020, 8:53 AM
sivachandra marked 2 inline comments as done.
sivachandra added inline comments.
libc/config/linux/api.td
319

Sorry, I am not sure what I was thinking then.

libc/spec/posix.td
186

Agreed.

libc/test/src/unistd/write_test.cpp
20

Nitty nit: May be create a var for "hello".

This revision is now accepted and ready to land.Apr 17 2020, 8:53 AM
abrachet updated this revision to Diff 258360.Apr 17 2020, 10:10 AM
abrachet marked 2 inline comments as done.

Move "hello" to a variable

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2020, 10:48 AM