This is an archive of the discontinued LLVM Phabricator instance.

WIP: [libc] Add fdopen for linux targets
Needs ReviewPublic

Authored by alfredfo on Jun 20 2023, 8:58 PM.

Details

Reviewers
michaelrj
Summary

This is a draft/work-in-progress. Probably needs some tests, and the
entry in libc/config is just an example here. Should also be added to
other arches.

Did this to get a little more comfortable with how llvm libc works,
comments are very welcome.

Diff Detail

Event Timeline

alfredfo created this revision.Jun 20 2023, 8:58 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 20 2023, 8:58 PM

Maybe fopen() should be implemented using fdopen()? It does the same thing but additionally calls the 'open' syscall to get a file descriptor to a file.

alfredfo published this revision for review.Aug 10 2023, 4:31 PM
alfredfo added inline comments.
libc/spec/stdc.td
535 ↗(On Diff #533120)

should be in posix.td

alfredfo updated this revision to Diff 550494.Aug 15 2023, 3:00 PM

make openfile(char*,char*) use new overload

alfredfo updated this revision to Diff 550599.Aug 15 2023, 9:17 PM

refactor into separate allocatefile function

alfredfo updated this revision to Diff 550602.Aug 15 2023, 9:33 PM

add a simple test (more needed)

This overall OK. But, I would like to spend some more time thinking about the new abstractions.

libc/src/__support/File/linux/file.cpp
105

This function can move into the anonymous namespace?

134

Can you add comments explaining why APPEND is handled as a special case?

libc/test/src/stdio/fdopen_test.cpp
29 ↗(On Diff #550602)

You should include few negative tests, say calling fdopen with a bad fd.