From 23d7a4c430cb6bc6c0bceac7d5ff7599a965c9db Mon Sep 17 00:00:00 2001 From: Simon Krueger Date: Fri, 8 Nov 2024 10:14:25 -0800 Subject: [PATCH] Declare folly::fileops for open, close, read, write, and pipe Summary: This creates `folly::fileops` for `open`, `close`, `read`, `write`, and `pipe` that will eventually replace the versions that folly places in the global scope. Folly's implementation of these functions differs from the existing Windows CRT version. `open`, `read`, `write`, and `close` add support for sockets. `pipe` uses sockets so libevent is supported. Folly's other posix functions that are absent from windows CRT (e.g., `fcntl`) remain available in the global scope via a `using` directive. `stdlib` and `sysstat` function declarations are moved from the global namespace to the `folly::portability::{stdlib, sysstat}` namespaces. This is the 1st phase in a 3-phase change to remove folly's global declarations of posix functions that conflict with the windows CRT. The 2nd phase updates the usage of these functions to use qualified name lookup (e.g., calls to `open` and changed to `folly::fileops::open`). The 3rd and final phase will remove folly's globally defined posix functions. Another round of work will move `folly:fileops` to another buck target cxx_library. **What is the reason for this change?** Folly's global definitions of posix functions on Windows causes `#include` order issues if folly is not included first. For example, when `gtest/gtest.h` is included before folly, gtest includes `windows.h` and that declares `open`, `read`, and `chdir`, which creates ambiguous references to folly's `open`, `read`, and `chdir`. Another example, is where posix functions go undeclared when `folly/portability/windows.h` is included without other portability headers (e.g., `folly/portability/unistd.h`). `folly/portability/windows.h` includes `windows.h` so that only underscore versions of the posix functions are available (e.g., `_open`, `_close`). These issues create friction for windows development. **Background: What is the purpose of `folly::portability::{fcntl,stdlib,sysstat,unistd}`?** It is a portability layer to make posix functions available and behave consistently across platforms. Some posix functions don't exist on windows (e.g., `sysconf`). Some other posix functions, folly changes to adapt behavior across platforms. For example, on windows folly defines `open`, `read`, `write`, and `close` functions to work with sockets. Folly makes these functions available in the global scope for convenience. Reviewed By: Orvid Differential Revision: D65153511 fbshipit-source-id: 557b96f3615b04ec80b53077089d88aa4bce5e54 --- folly/portability/BUCK | 2 ++ folly/portability/Fcntl.h | 10 ++++++++++ folly/portability/Stdlib.cpp | 10 +++++++--- folly/portability/Stdlib.h | 31 ++++++++++++++++++++++++------- folly/portability/SysStat.cpp | 8 ++++++-- folly/portability/SysStat.h | 15 +++++++++++++-- folly/portability/Unistd.h | 16 ++++++++++++++++ 7 files changed, 78 insertions(+), 14 deletions(-) diff --git a/folly/portability/BUCK b/folly/portability/BUCK index c7fd0b4bdf2..b191b0c989b 100755 --- a/folly/portability/BUCK +++ b/folly/portability/BUCK @@ -295,6 +295,7 @@ cpp_library( ], exported_deps = [ ":config", + "//folly:c_portability", ], ) @@ -366,6 +367,7 @@ cpp_library( ], exported_deps = [ ":sys_types", + "//folly:c_portability", ], ) diff --git a/folly/portability/Fcntl.h b/folly/portability/Fcntl.h index 8b5759f7bd4..eb149e130a4 100644 --- a/folly/portability/Fcntl.h +++ b/folly/portability/Fcntl.h @@ -65,3 +65,13 @@ FOLLY_POP_WARNING #else #define FOLLY_PORT_WIN32_OPEN_BINARY 0 #endif + +namespace folly { +namespace fileops { +#ifdef _WIN32 +using portability::fcntl::open; +#else +using ::open; +#endif +} // namespace fileops +} // namespace folly diff --git a/folly/portability/Stdlib.cpp b/folly/portability/Stdlib.cpp index e77a895efb7..53602dfeaec 100644 --- a/folly/portability/Stdlib.cpp +++ b/folly/portability/Stdlib.cpp @@ -26,7 +26,9 @@ #include #include -extern "C" { +namespace folly { +namespace portability { +namespace stdlib { char* mktemp(char* tn) { return _mktemp(tn); } @@ -138,7 +140,9 @@ int unsetenv(const char* name) { } return 0; } -} +} // namespace stdlib +} // namespace portability +} // namespace folly #endif @@ -147,7 +151,7 @@ int unsetenv(const char* name) { #include #include -extern "C" int clearenv() { +int folly::portability::stdlib::clearenv() { std::vector data; for (auto it = environ; it && *it; ++it) { std::string entry(*it); diff --git a/folly/portability/Stdlib.h b/folly/portability/Stdlib.h index c9d8f97e3ff..acba5d187c3 100644 --- a/folly/portability/Stdlib.h +++ b/folly/portability/Stdlib.h @@ -18,6 +18,7 @@ #include +#include #include #if defined(__APPLE__) @@ -39,12 +40,6 @@ extern "C" { #define NAME_MAX _MAX_FNAME #define HOST_NAME_MAX 255 -char* mktemp(char* tn); -char* mkdtemp(char* tn); -int mkstemp(char* tn); -char* realpath(const char* path, char* resolved_path); -int setenv(const char* name, const char* value, int overwrite); -int unsetenv(const char* name); #elif defined(__APPLE__) // environ doesn't work well with dylibs, so use _NSGetEnviron instead. #if !__has_include() @@ -57,8 +52,30 @@ char*** _NSGetEnviron(void); // Needed to resolve linkage extern char** environ; #endif +} +namespace folly { +namespace portability { +namespace stdlib { #if !__linux__ && !FOLLY_MOBILE int clearenv(); #endif -} + +#ifdef _WIN32 +char* mktemp(char* tn); +char* mkdtemp(char* tn); +int mkstemp(char* tn); +char* realpath(const char* path, char* resolved_path); +int setenv(const char* name, const char* value, int overwrite); +int unsetenv(const char* name); +#endif +} // namespace stdlib +} // namespace portability +} // namespace folly + +#if defined(_WIN32) || (!__linux__ && !FOLLY_MOBILE) +FOLLY_PUSH_WARNING +FOLLY_CLANG_DISABLE_WARNING("-Wheader-hygiene") +/* using override */ using namespace folly::portability::stdlib; +FOLLY_POP_WARNING +#endif diff --git a/folly/portability/SysStat.cpp b/folly/portability/SysStat.cpp index 21441f0560e..5db05f6b5d1 100644 --- a/folly/portability/SysStat.cpp +++ b/folly/portability/SysStat.cpp @@ -19,7 +19,9 @@ #ifdef _WIN32 #include -extern "C" { +namespace folly { +namespace portability { +namespace sysstat { int chmod(char const* fn, int am) { return _chmod(fn, am); } @@ -62,5 +64,7 @@ int mkdir(const char* fn, int /* mode */) { int umask(int md) { return _umask(md); } -} +} // namespace sysstat +} // namespace portability +} // namespace folly #endif diff --git a/folly/portability/SysStat.h b/folly/portability/SysStat.h index 88236865109..0a6c0bd2fee 100644 --- a/folly/portability/SysStat.h +++ b/folly/portability/SysStat.h @@ -18,6 +18,8 @@ #include +#include + #ifdef _WIN32 #include @@ -42,11 +44,20 @@ // This isn't defined anywhere, so give a sane value. #define MAXSYMLINKS 255 -extern "C" { +namespace folly { +namespace portability { +namespace sysstat { int chmod(char const* fn, int am); int fchmod(int fd, mode_t mode); int lstat(const char* path, struct stat* st); int mkdir(const char* fn, int mode); int umask(int md); -} +} // namespace sysstat +} // namespace portability +} // namespace folly + +FOLLY_PUSH_WARNING +FOLLY_CLANG_DISABLE_WARNING("-Wheader-hygiene") +/* using override */ using namespace folly::portability::sysstat; +FOLLY_POP_WARNING #endif diff --git a/folly/portability/Unistd.h b/folly/portability/Unistd.h index 1b8dd7484fe..91e664e8c70 100644 --- a/folly/portability/Unistd.h +++ b/folly/portability/Unistd.h @@ -109,3 +109,19 @@ FOLLY_CLANG_DISABLE_WARNING("-Wheader-hygiene") /* using override */ using namespace folly::portability::unistd; FOLLY_POP_WARNING #endif + +namespace folly { +namespace fileops { +#ifdef _WIN32 +using folly::portability::unistd::close; +using folly::portability::unistd::pipe; +using folly::portability::unistd::read; +using folly::portability::unistd::write; +#else +using ::close; +using ::pipe; +using ::read; +using ::write; +#endif +} // namespace fileops +} // namespace folly