Skip to content

Commit

Permalink
Declare folly::fileops for open, close, read, write, and pipe
Browse files Browse the repository at this point in the history
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
  • Loading branch information
skrueger authored and facebook-github-bot committed Nov 8, 2024
1 parent cf549a4 commit 23d7a4c
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 14 deletions.
2 changes: 2 additions & 0 deletions folly/portability/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ cpp_library(
],
exported_deps = [
":config",
"//folly:c_portability",
],
)

Expand Down Expand Up @@ -366,6 +367,7 @@ cpp_library(
],
exported_deps = [
":sys_types",
"//folly:c_portability",
],
)

Expand Down
10 changes: 10 additions & 0 deletions folly/portability/Fcntl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 7 additions & 3 deletions folly/portability/Stdlib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
#include <folly/portability/SysStat.h>
#include <folly/portability/Windows.h>

extern "C" {
namespace folly {
namespace portability {
namespace stdlib {
char* mktemp(char* tn) {
return _mktemp(tn);
}
Expand Down Expand Up @@ -138,7 +140,9 @@ int unsetenv(const char* name) {
}
return 0;
}
}
} // namespace stdlib
} // namespace portability
} // namespace folly

#endif

Expand All @@ -147,7 +151,7 @@ int unsetenv(const char* name) {
#include <string>
#include <vector>

extern "C" int clearenv() {
int folly::portability::stdlib::clearenv() {
std::vector<std::string> data;
for (auto it = environ; it && *it; ++it) {
std::string entry(*it);
Expand Down
31 changes: 24 additions & 7 deletions folly/portability/Stdlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <cstdlib>

#include <folly/CPortability.h>
#include <folly/portability/Config.h>

#if defined(__APPLE__)
Expand All @@ -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(<crt_externs.h>)
Expand All @@ -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
8 changes: 6 additions & 2 deletions folly/portability/SysStat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
#ifdef _WIN32
#include <folly/portability/Windows.h>

extern "C" {
namespace folly {
namespace portability {
namespace sysstat {
int chmod(char const* fn, int am) {
return _chmod(fn, am);
}
Expand Down Expand Up @@ -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
15 changes: 13 additions & 2 deletions folly/portability/SysStat.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#include <sys/stat.h>

#include <folly/CPortability.h>

#ifdef _WIN32
#include <folly/portability/SysTypes.h>

Expand All @@ -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
16 changes: 16 additions & 0 deletions folly/portability/Unistd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 23d7a4c

Please sign in to comment.