[libc-commits] [PATCH] D77626: [libc] Add very basic stdio FILE and fwrite

[Siva Chandra via Phabricator via libc-commits]

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


================
Comment at: libc/config/linux/api.td:23
+  let Decl = [{
+    typedef struct {} FILE;
+  }];
----------------
I understand all public API deals with `FILE*`. But, would an empty struct trigger an incorrect optimization in user code somewhere? I have no idea, but just checking since such a thought comes to my mind.


================
Comment at: libc/config/linux/api.td:165
   let Functions = [
-    "snprintf",
   ];
----------------
I added this as an example long time back and never checked if it works presently.


================
Comment at: libc/src/stdio/FILE.h:20
+
+struct FILE {
+  mtx_t lock;
----------------
May be this struct is scalable. As in, this structure can handle all kinds of FILE objects. But, few questions below.


================
Comment at: libc/src/stdio/FILE.h:23
+
+  void *cookie;
+  cookie_write_function_t *write;
----------------
For a simple use like this, a name like `cookie` seems alright. But, would it make sense to call it `write_ptr`?


================
Comment at: libc/src/stdio/FILE.h:24
+  void *cookie;
+  cookie_write_function_t *write;
+};
----------------
Should we actually specify the type of the writing function? Let it be set by the FILE creation function to the appropriate writer type?


================
Comment at: libc/test/config/linux/x86_64/syscall_test.cpp:1
 //===------------------ Unittests for x86_64 syscalls ---------------------===//
 //
----------------
Can you move the changes in this file to a separate patch?


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

https://reviews.llvm.org/D77626