WIP: [libc] Add fdopen for linux targets

[alfredfo]

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.

[alfredfo]

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]

libc/spec/stdc.td

535 ↗

(On Diff #533120)

should be in posix.td

[alfredfo]

u

[alfredfo]

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

[alfredfo]

refactor into separate allocatefile function

[alfredfo]

add a simple test (more needed)

[sivachandra]

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.

libc/config/linux/x86_64/entrypoints.txt

list(APPEND TARGET_LIBC_ENTRYPOINTS
     # setjmp.h entrypoints
     libc.src.setjmp.longjmp
     libc.src.setjmp.setjmp
 
     # stdio.h entrypoints
     libc.src.stdio.clearerr
     libc.src.stdio.clearerr_unlocked
     libc.src.stdio.fclose
+    libc.src.stdio.fdopen
     libc.src.stdio.flockfile
     libc.src.stdio.feof
     libc.src.stdio.feof_unlocked
     libc.src.stdio.ferror
     libc.src.stdio.ferror_unlocked
     libc.src.stdio.fgetc
     libc.src.stdio.fgetc_unlocked
     libc.src.stdio.fgets

libc/spec/posix.td

HeaderSpec StdIO = HeaderSpec<
               [ArgSpec<FILEPtr>]
           >,
           FunctionSpec<
               "funlockfile",
               RetValSpec<VoidType>,
               [ArgSpec<FILEPtr>]
           >,
           FunctionSpec<
+              "fdopen",
+              RetValSpec<FILEPtr>,
+              [ArgSpec<IntType>,
+               ArgSpec<ConstCharPtr>]
+          >,
+          FunctionSpec<
               "getc_unlocked",
               RetValSpec<IntType>,
               [ArgSpec<FILEPtr>]
           >,
           FunctionSpec<
               "getchar_unlocked",
               RetValSpec<IntType>,
               [ArgSpec<VoidType>]

libc/src/__support/File/file.h

if (read_allowed() && (buf == nullptr || bufsize == 0)) {
       own_buf = false; // We shouldn't call free on |buf| when closing the file.
     }
   }
 };
 
 // The implementaiton of this function is provided by the platfrom_file
 // library.
 ErrorOr<File *> openfile(const char *path, const char *mode);
+ErrorOr<File *> openfile(int fd, const char *mode);
 
 // The platform_file library should implement it if it relevant for that
 // platform.
 int get_fileno(File *f);
 
 extern File *stdin;
 extern File *stdout;
 extern File *stderr;
 
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_SUPPORT_OSUTIL_FILE_H

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

if (ret < 0) {
     return -ret;
   }
   delete lf;
   return 0;
 }
 
 } // anonymous namespace
 
+ErrorOr<File *> allocatefile(int fd, File::ModeFlags modeflags) {
+  if (fd < 0)
+    return Error(-fd);
+
+  uint8_t *buffer;
+  {
+    AllocChecker ac;
+    buffer = new (ac) uint8_t[File::DEFAULT_BUFFER_SIZE];
+    if (!ac)
+      return Error(ENOMEM);
+  }
+  AllocChecker ac;
+  auto *file = new (ac)
+      LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
+  if (!ac)
+    return Error(ENOMEM);
+  return file;
+}
+
+ErrorOr<File *> openfile(int fd, const char *mode) {
+  if (fd < 0)
+    return Error(EBADF);
+  using ModeFlags = File::ModeFlags;
+  auto modeflags = File::mode_flags(mode);
+  if (modeflags == 0) {
+    // return {nullptr, EINVAL};
+    return Error(EINVAL);
+  }
+
+  if (modeflags & ModeFlags(File::OpenMode::APPEND)) {
+    auto cur_flags = __llvm_libc::syscall_impl<int>(SYS_fcntl, fd, F_GETFL);
+    if (!(cur_flags & O_APPEND))
+      __llvm_libc::syscall_impl<int>(SYS_fcntl, fd, F_SETFL,
+                                     cur_flags | O_APPEND);
+  }
+  return allocatefile(fd, modeflags);
+}
+
 ErrorOr<File *> openfile(const char *path, const char *mode) {
   using ModeFlags = File::ModeFlags;
   auto modeflags = File::mode_flags(mode);
   if (modeflags == 0) {
     // return {nullptr, EINVAL};
     return Error(EINVAL);
   }
   long open_flags = 0;
int fd =
       __llvm_libc::syscall_impl<int>(SYS_open, path, open_flags, OPEN_MODE);
 #elif defined(SYS_openat)
   int fd = __llvm_libc::syscall_impl<int>(SYS_openat, AT_FDCWD, path,
                                           open_flags, OPEN_MODE);
 #else
 #error "open and openat syscalls not available."
 #endif
 
-  if (fd < 0)
-    return Error(-fd);
-
-  uint8_t *buffer;
-  {
-    AllocChecker ac;
-    buffer = new (ac) uint8_t[File::DEFAULT_BUFFER_SIZE];
-    if (!ac)
-      return Error(ENOMEM);
-  }
-  AllocChecker ac;
-  auto *file = new (ac)
-      LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
-  if (!ac)
-    return Error(ENOMEM);
-  return file;
+  return allocatefile(fd, modeflags);
 }
 
 int get_fileno(File *f) {
   auto *lf = reinterpret_cast<LinuxFile *>(f);
   return lf->get_fd();
 }
 
 constexpr size_t STDIN_BUFFER_SIZE = 512;

libc/src/stdio/CMakeLists.txt

HDRS
     fopen.h
   DEPENDS
     libc.include.stdio
     libc.src.__support.File.file
     libc.src.__support.File.platform_file
 )
 
 add_entrypoint_object(
+  fdopen
+  SRCS
+    fdopen.cpp
+  HDRS
+    fdopen.h
+  DEPENDS
+    libc.include.stdio
+    libc.src.__support.File.file
+    libc.src.__support.File.platform_file
+)
+
+add_entrypoint_object(
   fclose
   SRCS
     fclose.cpp
   HDRS
     fclose.h
   DEPENDS
     libc.include.stdio
     libc.src.errno.errno

libc/src/stdio/fdopen.h

+//===-- Implementation header of fdopen --------------------------*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDIO_FDOPEN_H
+#define LLVM_LIBC_SRC_STDIO_FDOPEN_H
+
+#include <stdio.h>
+
+namespace __llvm_libc {
+
+::FILE *fdopen(int fd, const char *__restrict mode);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STDIO_FDOPEN_H

libc/src/stdio/fdopen.cpp

+//===-- Implementation of fdopen
+//-------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdio/fdopen.h"
+#include "src/__support/File/file.h"
+
+#include "src/errno/libc_errno.h"
+#include <stdio.h>
+
+namespace __llvm_libc {
+
+LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *__restrict mode)) {
+  auto result = __llvm_libc::openfile(fd, mode);
+  if (!result.has_value()) {
+    libc_errno = result.error();
+    return nullptr;
+  }
+  return reinterpret_cast<::FILE *>(result.value());
+}
+
+} // namespace __llvm_libc