Skip to content

Commit

Permalink
JSBigString: Fix building on Windows (#26826)
Browse files Browse the repository at this point in the history 
Summary:
This pull request replaces the last remaining Unix headers in `JSBigString` with their equivalent Folly Portability headers, and replaces the calls to `getpagesize()` with `sysconf(_SC_PAGESIZE)` since Folly Portability is missing that function.

The work to get this building on windows was mostly done by acoates-ms, this pull request just adds the finishing touches.

## Changelog:

[General] [Fixed] - Fixed `JSBigString` not compiling on Windows due to Unix-specific headers
Pull Request resolved: #26826

Test Plan: Compiled with Clang and with MSVC (2017)

Differential Revision: D17903214

Pulled By: cpojer

fbshipit-source-id: 230f8fb410fa81d8f13d8b6ccf1147cfc70358bf
  • Loading branch information
@empyrical @facebook-github-bot
empyrical authored and facebook-github-bot committed Oct 14, 2019
1 parent a0d8740 commit 80857f2
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 69 deletions.
63 changes: 30 additions & 33 deletions ReactCommon/cxxreact/JSBigString.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,28 @@

#include "JSBigString.h"

#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>

#include <glog/logging.h>

#include <folly/Memory.h>
#include <folly/portability/SysMman.h>
#include <folly/ScopeGuard.h>
#include <folly/portability/Fcntl.h>
#include <folly/portability/SysMman.h>
#include <folly/portability/SysStat.h>
#include <folly/portability/Unistd.h>

namespace facebook {
namespace react {

JSBigFileString::JSBigFileString(int fd, size_t size, off_t offset /*= 0*/)
: m_fd { -1 }
, m_data { nullptr } {
folly::checkUnixError(m_fd = dup(fd),
"Could not duplicate file descriptor");
: m_fd{-1}, m_data{nullptr} {
folly::checkUnixError(m_fd = dup(fd), "Could not duplicate file descriptor");

// Offsets given to mmap must be page aligned. We abstract away that
// restriction by sending a page aligned offset to mmap, and keeping track
// of the offset within the page that we must alter the mmap pointer by to
// get the final desired offset.
if (offset != 0) {
const static auto ps = getpagesize();
const static auto ps = sysconf(_SC_PAGESIZE);
auto d = lldiv(offset, ps);

m_mapOff = d.quot;
Expand Down Expand Up @@ -69,8 +66,7 @@ static off_t maybeRemap(char *data, size_t size, int fd) {
}
const auto begin = data;
static const uint8_t kRemapMagic[] = {
0xc6, 0x1f, 0xbc, 0x03, 0xc1, 0x03, 0x19, 0x1f, 0xa1, 0xd0, 0xeb, 0x73
};
0xc6, 0x1f, 0xbc, 0x03, 0xc1, 0x03, 0x19, 0x1f, 0xa1, 0xd0, 0xeb, 0x73};
if (::memcmp(data, kRemapMagic, sizeof(kRemapMagic)) != 0) {
return 0;
}
Expand All @@ -82,26 +78,26 @@ static off_t maybeRemap(char *data, size_t size, int fd) {
{
// System page size must be at least as granular as the remapping.
// TODO: Consider fallback that reads entire file into memory.
const size_t systemPS = getpagesize();
const size_t systemPS = sysconf(_SC_PAGESIZE);
CHECK(filePS >= systemPS)
<< "filePS: " << filePS
<< "systemPS: " << systemPS;
<< "filePS: " << filePS << "systemPS: " << systemPS;
}
const off_t headerPages = read16(data);
uint16_t numMappings = read16(data);
size_t curFilePage = headerPages;
while (numMappings--) {
auto memPage = read16(data) + headerPages;
auto numPages = read16(data);
if (mmap(begin + memPage * filePS, numPages * filePS,
PROT_READ, MAP_FILE | MAP_PRIVATE | MAP_FIXED,
fd, curFilePage * filePS) == MAP_FAILED) {
CHECK(false)
<< " memPage: " << memPage
<< " numPages: " << numPages
<< " curFilePage: " << curFilePage
<< " size: " << size
<< " error: " << std::strerror(errno);
if (mmap(
begin + memPage * filePS,
numPages * filePS,
PROT_READ,
MAP_FILE | MAP_PRIVATE | MAP_FIXED,
fd,
curFilePage * filePS) == MAP_FAILED) {
CHECK(false) << " memPage: " << memPage << " numPages: " << numPages
<< " curFilePage: " << curFilePage << " size: " << size
<< " error: " << std::strerror(errno);
}
curFilePage += numPages;
}
Expand All @@ -115,12 +111,10 @@ const char *JSBigFileString::c_str() const {
}
if (!m_data) {
m_data =
(const char *) mmap(0, m_size, PROT_READ, MAP_PRIVATE, m_fd, m_mapOff);
(const char *)mmap(0, m_size, PROT_READ, MAP_PRIVATE, m_fd, m_mapOff);
CHECK(m_data != MAP_FAILED)
<< " fd: " << m_fd
<< " size: " << m_size
<< " offset: " << m_mapOff
<< " error: " << std::strerror(errno);
<< " fd: " << m_fd << " size: " << m_size << " offset: " << m_mapOff
<< " error: " << std::strerror(errno);
#ifdef WITH_FBREMAP
// Remapping is only attempted when the entire file was requested.
if (m_mapOff == 0 && m_pageOff == 0) {
Expand All @@ -141,16 +135,19 @@ int JSBigFileString::fd() const {
return m_fd;
}

std::unique_ptr<const JSBigFileString> JSBigFileString::fromPath(const std::string& sourceURL) {
std::unique_ptr<const JSBigFileString> JSBigFileString::fromPath(
const std::string &sourceURL) {
int fd = ::open(sourceURL.c_str(), O_RDONLY);
folly::checkUnixError(fd, "Could not open file", sourceURL);
SCOPE_EXIT { CHECK(::close(fd) == 0); };
SCOPE_EXIT {
CHECK(::close(fd) == 0);
};

struct stat fileInfo;
folly::checkUnixError(::fstat(fd, &fileInfo), "fstat on bundle failed.");

return folly::make_unique<const JSBigFileString>(fd, fileInfo.st_size);
}

} // namespace react
} // namespace facebook
} // namespace react
} // namespace facebook
67 changes: 31 additions & 36 deletions ReactCommon/cxxreact/JSBigString.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@

#pragma once

#include <fcntl.h>
#include <sys/mman.h>

#include <folly/Exception.h>

#ifndef RN_EXPORT
# ifdef _MSC_VER
# define RN_EXPORT
# else
# define RN_EXPORT __attribute__((visibility("default")))
# endif
#ifdef _MSC_VER
#define RN_EXPORT
#else
#define RN_EXPORT __attribute__((visibility("default")))
#endif
#endif

namespace facebook {
Expand All @@ -29,19 +26,19 @@ namespace react {
// by CopyConstructible.

class JSBigString {
public:
public:
JSBigString() = default;

// Not copyable
JSBigString(const JSBigString&) = delete;
JSBigString& operator=(const JSBigString&) = delete;
JSBigString(const JSBigString &) = delete;
JSBigString &operator=(const JSBigString &) = delete;

virtual ~JSBigString() {}

virtual bool isAscii() const = 0;

// This needs to be a \0 terminated string
virtual const char* c_str() const = 0;
virtual const char *c_str() const = 0;

// Length of the c_str without the NULL byte.
virtual size_t size() const = 0;
Expand All @@ -50,24 +47,23 @@ class JSBigString {
// Concrete JSBigString implementation which holds a std::string
// instance.
class JSBigStdString : public JSBigString {
public:
JSBigStdString(std::string str, bool isAscii=false)
: m_isAscii(isAscii)
, m_str(std::move(str)) {}
public:
JSBigStdString(std::string str, bool isAscii = false)
: m_isAscii(isAscii), m_str(std::move(str)) {}

bool isAscii() const override {
return m_isAscii;
}

const char* c_str() const override {
const char *c_str() const override {
return m_str.c_str();
}

size_t size() const override {
return m_str.size();
}

private:
private:
bool m_isAscii;
std::string m_str;
};
Expand All @@ -77,10 +73,8 @@ class JSBigStdString : public JSBigString {
// used to construct a JSBigString in place, such as by reading from a
// file.
class RN_EXPORT JSBigBufferString : public JSBigString {
public:
JSBigBufferString(size_t size)
: m_data(new char[size + 1])
, m_size(size) {
public:
JSBigBufferString(size_t size) : m_data(new char[size + 1]), m_size(size) {
// Guarantee nul-termination. The caller is responsible for
// filling in the rest of m_data.
m_data[m_size] = '\0';
Expand All @@ -94,27 +88,26 @@ class RN_EXPORT JSBigBufferString : public JSBigString {
return true;
}

const char* c_str() const override {
const char *c_str() const override {
return m_data;
}

size_t size() const override {
return m_size;
}

char* data() {
char *data() {
return m_data;
}

private:
char* m_data;
private:
char *m_data;
size_t m_size;
};

// JSBigString interface implemented by a file-backed mmap region.
class RN_EXPORT JSBigFileString : public JSBigString {
public:

public:
JSBigFileString(int fd, size_t size, off_t offset = 0);
~JSBigFileString();

Expand All @@ -127,14 +120,16 @@ class RN_EXPORT JSBigFileString : public JSBigString {
size_t size() const override;
int fd() const;

static std::unique_ptr<const JSBigFileString> fromPath(const std::string& sourceURL);
static std::unique_ptr<const JSBigFileString> fromPath(
const std::string &sourceURL);

private:
int m_fd; // The file descriptor being mmaped
size_t m_size; // The size of the mmaped region
mutable off_t m_pageOff; // The offset in the mmaped region to the data.
off_t m_mapOff; // The offset in the file to the mmaped region.
mutable const char *m_data; // Pointer to the mmaped region.
private:
int m_fd; // The file descriptor being mmaped
size_t m_size; // The size of the mmaped region
mutable off_t m_pageOff; // The offset in the mmaped region to the data.
off_t m_mapOff; // The offset in the file to the mmaped region.
mutable const char *m_data; // Pointer to the mmaped region.
};

} }
} // namespace react
} // namespace facebook

0 comments on commit 80857f2

Please sign in to comment.