[libc-commits] [PATCH] D149907: [libc] add scanf system file support

[Roland McGrath via Phabricator via libc-commits]

mcgrathr added inline comments.


================
Comment at: libc/src/stdio/fscanf.cpp:21
+#else  // defined(LIBC_COPT_SCANF_USE_SYSTEM_FILE)
+using FileT = ::FILE;
+#endif // LIBC_COPT_SCANF_USE_SYSTEM_FILE
----------------
This is rather different from what I was expecting. I hadn't seen D146001 before now, so I was thinking of the integration with FILE or whatever other underlying i/o mechanism as being entirely provided by the integrator. It's not clear to me if we have full parity here between the printf and scanf cores. That seems desirable, i.e. that a purely abstract interface can be filled out unrelated to anybody's concept of a `FILE` type, for the i/o interactions that the core needs. For integrating with a particular stdio implementation, it's not clear whether using the public stdio interfaces will be what's most desirable. The integrator should be able to write things up to their own internals, whether or not those look at all like what you presume stdio looks like.


================
Comment at: libc/src/stdio/fscanf.cpp:35
   va_end(vlist);
-  int ret_val = scanf_core::vfscanf_internal(stream, format, args);
+  int ret_val = scanf_core::vfscanf_internal(reinterpret_cast<FileT *>(stream),
+                                             format, args);
----------------
ISTM that the integrator-provided template instantiations should be responsible for `FILE*`->`FileT*` rather than presuming that a `reinterpret_cast` here is safe and correct.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149907