Skip to content

Commit

Permalink
Use folly::fileops qualified name lookup
Browse files Browse the repository at this point in the history
Summary:
This is a codemod. It was automatically generated and will be landed once it is approved and tests are passing in sandcastle.
You have been added as a reviewer by Sentinel or Butterfly.

Autodiff project: fileops2
Autodiff partition: fbcode.watchman
Autodiff bookmark: ad.fileops2.fbcode.watchman

This updates `open`, `close`, `read`, `write`, and `pipe` call sites to use
`folly::fileops` qualified name lookup.

This is the 2nd phase in a 3-phase change to remove folly's global definitions
of the posix functions that conflict with windows CRT.
The 1st phase created namespaces for folly's posix functions. The 2nd phase
updates callsites to use the qualified name of folly's  `open`, `close`,
`read`, `write`, and `pipe`  functions. The 3rd and final phase will remove
folly's globally defined posix functions and have windows CRT define them
again.

**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` in a way 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: MichaelCuevas

Differential Revision: D65855144

fbshipit-source-id: 2c9df0d0ee833a8c063798e360f29e6b93f90683
  • Loading branch information
skrueger authored and facebook-github-bot committed Nov 14, 2024
1 parent c932fb2 commit 1038556
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 6 deletions.
5 changes: 3 additions & 2 deletions watchman/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ namespace {
template <typename String>
void write_stderr(const String& str) {
w_string_piece piece = str;
ignore_result(::write(STDERR_FILENO, piece.data(), piece.size()));
ignore_result(
folly::fileops::write(STDERR_FILENO, piece.data(), piece.size()));
}

template <typename String, typename... Strings>
Expand Down Expand Up @@ -202,7 +203,7 @@ void Log::doLogToStdErr() {

for (auto& item : items) {
auto& log = json_to_w_string(item->payload.get("log"));
ignore_result(::write(STDERR_FILENO, log.data(), log.size()));
ignore_result(folly::fileops::write(STDERR_FILENO, log.data(), log.size()));

auto level = json_to_w_string(item->payload.get("level"));
if (level == kFatal) {
Expand Down
2 changes: 1 addition & 1 deletion watchman/Logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ void logf(LogLevel level, fmt::string_view format_str, Args&&... args) {
template <typename... Args>
void logf_stderr(fmt::string_view format_str, Args&&... args) {
auto msg = fmt::format(fmt::runtime(format_str), std::forward<Args>(args)...);
ignore_result(write(STDERR_FILENO, msg.data(), msg.size()));
ignore_result(folly::fileops::write(STDERR_FILENO, msg.data(), msg.size()));
}

#ifdef _WIN32
Expand Down
6 changes: 3 additions & 3 deletions watchman/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,15 @@ std::optional<std::string> detect_starting_command(pid_t ppid) {
int fd = ::open("/dev/null", O_RDONLY);
if (fd != -1) {
ignore_result(::dup2(fd, STDIN_FILENO));
::close(fd);
folly::fileops::close(fd);
}

if (logging::log_name != "-") {
fd = open(logging::log_name.c_str(), O_WRONLY | O_APPEND | O_CREAT, 0600);
if (fd != -1) {
ignore_result(::dup2(fd, STDOUT_FILENO));
ignore_result(::dup2(fd, STDERR_FILENO));
::close(fd);
folly::fileops::close(fd);
}
}

Expand Down Expand Up @@ -259,7 +259,7 @@ static void close_random_fds() {
}

for (max_fd = open_max; max_fd > STDERR_FILENO; --max_fd) {
close(max_fd);
folly::fileops::close(max_fd);
}
#endif
}
Expand Down

0 comments on commit 1038556

Please sign in to comment.